Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to create NamedTemporaryFile without the context manager? #161

Open
mateokurti opened this issue Feb 28, 2023 · 2 comments
Open

How to create NamedTemporaryFile without the context manager? #161

mateokurti opened this issue Feb 28, 2023 · 2 comments

Comments

@mateokurti
Copy link

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
Copy link
Owner

Tinche commented Feb 28, 2023

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
Copy link

mikenerone commented Mar 2, 2024

@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants