FileBinaryRead: file is already closed

[danjac]

Trying to run this command using FileBinaryRead e.g.

./manage.py my_command path_to_file.txt

app = Typer()

@app.command()
def handle(self, file: typer.FileBinaryRead) -> None:
    """Print out file"""
    for chunk in file:
        print(chunk)

ValueError: I/O operation on closed file

In the Typer docs this should work: https://typer.tiangolo.com/tutorial/parameter-types/file/#filebinaryread

[bckohan]

@danjac Thanks for the bug report!

I suspect this bug exists because django's BaseCommand has a two-phase parse/execute execution pathway and we've stuffed click's single phase pathway into it which results in some redundant work - for example two contexts are constructed. Click's file type closes the file on context destruction.

I really consider this a click design flaw because parse/execute should be independent exposed but chainable processes - but I doubt click is going to change at this point so we probably need to come up with a workaround. I do not want to inject special case code for these types because working equivalent behavior is easy (see below) and the most correct solution would be to avoid the redundant contexts. This will be more difficult to solve behind Django's interface, so I might not be able to get to it for a bit.

In the mean time it's probably best to avoid these types anyway. This implementation is a similar level of verbosity and also fully encapsulates file/open close. The above code puts the responsibility on the caller to close the file when invoked directly as a function (not through the CLI or call_command):

An equivalent example:

app = Typer()

@app.command()
def handle(self, file: Path) -> None:
    """Print out file"""
    for chunk in file.open("rb"):
        print(chunk)

An example that encapsulates handle ownership more fully:

app = Typer()

@app.command()
def handle(self, file: Path) -> None:
    """Print out file"""
    with open(file, "rb") as f:
        for chunk in f:
            print(chunk)

[bckohan]

Blocked by #210

[danjac]

Unfortunately I also need to be able to pipe streams to the command as well as use file paths, so for example cat some_file.txt | ./manage.py my_command - does not work with Path as argument.

[bckohan]

@danjac Weirdly the pipe use case actually works. I'll see what I can do.

[bckohan]

@danjac I don't want to rush this fix because it's delicate. In the meantime here's an easy workaround that works for the pipe and argument case:

app = Typer()

@app.command()
def handle(self, file: typer.FileBinaryRead) -> None:
    """Print out file"""
    if file.closed:
        file = open(file.name, "rb")
    for chunk in file:
        print(chunk)

[danjac]

Makes sense, thanks!