Tinche / aiofiles

File support for asyncio
Apache License 2.0
2.76k stars 150 forks source link

How to create NamedTemporaryFile without the context manager? #161

Open mateokurti opened 1 year ago

mateokurti commented 1 year ago

I have a helper function which creates a temporary file and returns it. That's how it is done synchronously:

def create_temp_file(file, suffix):
    temp_file = NamedTemporaryFile(suffix=suffix)
    temp_file.write(file.read())
    temp_file.seek(0)
    return temp_file

Now, I wanted to make it async and started using aiofiles.tempfile.NamedTemporaryFile instead. If I use the context manager as in the following snippet, the file can't be accessed as it is deleted after the with block:

async def create_temp_file_async(file, suffix):
    async with NamedTemporaryFileAsync(suffix=suffix) as temp_file:
        await temp_file.write(file.read())
        await temp_file.seek(0)
        return temp_file

If I try it with delete=False, it works, but the file is not deleted. Based on this comment, I managed to achieve the same result, but I'm not sure if this is the only and best way to do that:

async def create_temp_file_async(file, suffix):
    temp_file = await NamedTemporaryFileAsync(suffix=suffix).__aenter__()
    await temp_file.write(file.read())
    await temp_file.seek(0)
    return temp_file

Is this the right approach or is there some better way to do it?

Tinche commented 1 year ago

Yeah, that sounds about right, but you'll need to remember to await temp_file.__aexit__(None, None, None) when you're done with the file. (Or I guess you can let the garbage collector try handling it when it cleans up the file.)

mikenerone commented 7 months ago

@mateokurti

With the implementation as it is now, NamedTemporaryFileAsync is an async context manager. What's important is ensuring that it's __aexit__() method gets called when you're done with it. That's where the cleanup magic happens. I'd strongly suggest you do one of two things:

  1. The right way: Make your create_temp_file_async() itself return an async context manager. This allows it to internally use the NamedTemporaryFileAsync context manager properly. The __anter__() and __aexit__() calls then happen automatically like they're supposed to, so this looks like:
from contextlib import asynccontextmanager

@asynccontextmanager
async def create_temp_file_async(file, suffix):
    async with NamedTemporaryFileAsync(suffix=suffix) as temp_file:
        await temp_file.write(file.read())
        await temp_file.seek(0)
        yield temp_file  # Note `yield` instead of `return` here. Please read `contextlib` docs if you don't understand what this is doing.

# Example usage
async with create_temp_file_async(orig_file, ".tmp") as temp_file:
    await do_something_with_it_while_its_open(temp_file)
  1. Alternatively, you could (but really, don't do this) call __aexit__() yourself in the calling code. You need to make sure it's called, so do it in a try/finally.
# This is exactly your last example from the original comment
async def create_temp_file_async(file, suffix):
    temp_file = await NamedTemporaryFileAsync(suffix=suffix).__aenter__()
    await temp_file.write(file.read())
    await temp_file.seek(0)
    return temp_file

# Example usage
temp_file = create_temp_file_async(file, suffix)
try:
    await do_something_with_it_while_its_open(temp_file)
finally:
    await temp_file.__aexit__(None, None, None)

As you can see, the first approach encapsulates the interaction much better and is very Pythonic. As I said, you really shouldn't do the second one. By putting the onus on the caller, it's fragile and a terrible intermixing of concerns.

@Tinche One suggestion I'd have for aiofiles is to move the cleanup behavior to the standard aclose() async method, just as regular file objects have a synchronous close() method. Obviously __aexit__() would await self.aclose(). This provides an interface that parallels regular file objects, where the caller can simply call await async_file.aclose() when they choose to. I'd always recommend using it as a context manager, but giving people choices is good, and sometimes it's actually inconvenient because the close is far away from the open. It would also support the option of using contextlib.aclosing() from the stdlib to handle the close, which is something a lot of people are used to:

temp_file = create_temp_file_async(suffix=suffix)  # Again, assuming someone, for whatever reason, wants a non-CM wrapper like option 2 above.
async with aclosing(temp_file):
    await do_something_with_it_while_its_open(temp_file)