Add async support for temporary file handling. Closes #344 #867

11kkw · 2025-02-10T16:29:47Z

Changes

feat: Add async support for temporary file handling (#344)

Implemented async support for TemporaryFile, NamedTemporaryFile, SpooledTemporaryFile, and TemporaryDirectory.
Integrated to_thread for handling async operations with tempfiles.
Added tests to ensure async compatibility.

Checklist

You've added tests (in tests/) which would fail without your patch.
You've updated the documentation (in docs/, in case of behavior changes or new features). (Pending)
You've added a new changelog entry (in docs/versionhistory.rst). (Pending)

Updating the changelog

(Changelog not updated)

Additional Notes

I have implemented async support for temporary file handling.
Currently, I haven't updated the documentation or changelog yet.

I'm unsure if this implementation follows the best approach, so I would appreciate feedback on the current implementation first.
Once I receive input from the review, I will update the documentation and changelog accordingly if needed.

…#344

agronholm

Looks good in general, but I left several change requests.

tests/test_tempfile.py

src/anyio/_core/_tempfile.py

tests/test_tempfile.py

- Remove redundant decorators - Adjust type annotations by removing unnecessary parentheses - Import from its correct location - Remove unnecessary async versions of - Add blank lines for better readability after control blocks - Remove redundant type casts and isinstance checks - Group related tests under a test class for better organization - Fix path variable naming consistency - Ensure proper handling of blocking I/O within async functions

agronholm · 2025-02-11T18:09:00Z

You also need to fix the pyright errors.

src/anyio/_core/_tempfile.py

agronholm · 2025-02-11T18:20:55Z

Given that this is a new feature, I think it warrants a new documentation chapter. The code examples don't need to be comprehensive, but should make readers aware of this feature.

…nore ASYNC230 - Insert a blank line after the end of every control block to separate it from the following code (per review instructions). - Explicitly specify return types and adjust type annotations to resolve Pyright errors. - Maintain blocking I/O inside to_thread.run_sync() as per ASYNC230 compliance for runtime code. - Suppress ASYNC230 warnings in the test suite by adding per-file ignores in pyproject.toml.

11kkw · 2025-02-11T20:12:43Z

Given that this is a new feature, I think it warrants a new documentation chapter. The code examples don't need to be comprehensive, but should make readers aware of this feature.

I will add the document work and commit it as soon as possible. Thank you for your thorough review!

graingert · 2025-02-12T09:58:14Z

src/anyio/_core/_tempfile.py

+            self._async_file = None
+
+
+class SpooledTemporaryFile(Generic[AnyStr]):


Ideally for SpooledTemporaryFile is that writes/reads to/from the BytesIO buffer will not go to a thread

SpooledTemporaryFile needs to not go to a thread for these 'small' file things because the usecase is starlette multipart form file uploads which needs to remain fast for tiny uploads

@Kludex would you like to participate in the review of this feature too?

@11kkw I think @graingert is referring to something like this: https://github.com/Tinche/aiofiles/blob/main/src/aiofiles/tempfile/temptypes.py#L25-L55

I've made the necessary changes based on your feedback, but I'm not entirely sure if I've implemented them correctly due to my limited experience. I'd really appreciate it if you could review my changes and provide any further suggestions. Thank you for your guidance!"

This class needs an aclose()

Yep, it needs the aclose().

src/anyio/_core/_tempfile.py

- Optimized to avoid unnecessary thread switching for small files - Fixed blank lines after control blocks for better readability - Resolved Pyright type errors - Updated async tempfile documentation based on review feedback

src/anyio/_core/_tempfile.py

agronholm · 2025-02-13T16:42:34Z

Tests on 3.14 are failing because a new alpha added yet more attributes to pathlib.Path.

agronholm · 2025-02-13T16:43:20Z

Oh, before I forget: the API reference needs to be updated to include the new classes and functions.

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

…te API docs - Improved type annotations for mkstemp and mkdtemp functions using overloads per Typeshed. - Fixed test failures on Python 3.14 caused by additional attributes in pathlib.Path. - Updated API reference to include new classes and functions.

agronholm

I added some minor clean-ups as suggestions. The only real piece missing is the additions to the API reference documentation, and then @graingert needs to re-check the work you did on SpooledTemporaryFile. We're almost done now!

src/anyio/_core/_tempfile.py

tests/test_tempfile.py

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

docs/api.rst

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

graingert · 2025-02-17T11:45:17Z

src/anyio/_core/_tempfile.py

+        if not getattr(self._fp, "_rolled", True):
+            result = self._fp.write(s)
+            if self._fp._max_size and self._fp.tell() > self._fp._max_size:
+                self._fp.rollover()


Doesn't this do blocking IO?

write will do the rollover for us in this situation, which will result in blocking disk IO on the event loop there, so we need to make sure it won't.

You don't have to use SpooledTemporyFile here you can just copy the source code here with attribution, and modify it to use an async file

@11kkw Yeah, so instead of delegating to tempfile.SpooledTemporaryFile, @graingert is suggesting that you roll an independent implementation in order to get more control over async checkpoints and such.

graingert · 2025-02-17T11:49:13Z

src/anyio/_core/_tempfile.py

+    ) -> SpooledTemporaryFile[AnyStr]:
+        fp = await to_thread.run_sync(
+            partial(
+                tempfile.SpooledTemporaryFile,


Creating the SpooledTemporyFile doesn't perform blocking disk IO so this shouldn't need a thread

Right, you already mentioned that ^ here.

graingert · 2025-02-17T11:50:15Z

src/anyio/_core/_tempfile.py

+        self.dir: AnyStr | None = dir
+        self.errors = errors
+        self._async_file: AsyncFile | None = None
+        self._fp: Any = None


I think should be initialized here, how does the upstream SpooledTemporyFile behave?

It doesn't do any file I/O if that's what you mean, so yes, self._fp should be initialized to tempfile.SpooledTemporaryFile here.

graingert · 2025-02-17T11:52:08Z

src/anyio/_core/_tempfile.py

+            raise RuntimeError("Underlying file is not initialized.")
+
+        if not getattr(self._fp, "_rolled", True):
+            result = self._fp.write(s)


Suggested change

result = self._fp.write(s)

return self._fp.write(s)

graingert · 2025-02-17T11:55:51Z

pyproject.toml

@@ -90,6 +90,9 @@ extend-select = [
 [tool.ruff.lint.isort]
 "required-imports" = ["from __future__ import annotations"]

+[tool.ruff.lint.per-file-ignores]
+"tests/test_tempfile.py" = ["ASYNC230"]


Which open call is blocking?

The one in test_mkstemp().

graingert · 2025-02-17T11:56:41Z

tests/test_tempfile.py

+    async def test_exception_handling(self) -> None:
+        async with NamedTemporaryFile[bytes]() as af:
+            filename = af.name
+            assert os.path.exists(filename)  # type: ignore[arg-type]


Why is this a mypy error?

Because the name attribute is retrieved using __getattr__() which is declared to return object. I think this has been used elsewhere in anyio too – perhaps we should declare it to return Any instead?

Of course, that won't be necessary if this is just reimplemented by copying stdlib code.

graingert · 2025-02-17T11:59:53Z

src/anyio/_core/_tempfile.py

+            raise RuntimeError("Underlying file is not initialized.")
+
+        if not getattr(self._fp, "_rolled", True):
+            result = self._fp.write(s)


We need to have a bit of a think here. According to the rules of trio every async function should always checkpoint, however for SpooledTemporyFile to be useful it should be as close to a BytesIO in terms of performance at small sizes. The documentation should have a big red note here or write should include a checkpoint if we're writing to the memory buffer

agronholm · 2025-02-17T23:40:54Z

tests/test_tempfile.py

+
+
+@pytest.mark.parametrize("prefix", [b"mkdtemp_", "mkdtemp_"])
+async def test_mkdtemp(prefix: AnyStr) -> None:


The typevar is unwarranted here.

Suggested change

async def test_mkdtemp(prefix: AnyStr) -> None:

async def test_mkdtemp(prefix: str | bytes) -> None:

agronholm · 2025-02-18T17:27:09Z

@11kkw Let us know if you need help. I understand all this back and forth may feel pretty exhausting!

Kludex · 2025-02-22T16:37:02Z

src/anyio/_core/_tempfile.py

+            self._async_file = None
+
+
+class SpooledTemporaryFile(Generic[AnyStr]):


Why is this a generic? The tempfile.SpooledTemporaryFile is not a generic.

Looks pretty generic to me? https://github.com/python/typeshed/blob/ac8f2632ec37bb4a82ade0906e6ce9bdb33883d3/stdlib/tempfile.pyi#L277

There's something different happening here...

from tempfile import SpooledTemporaryFile from anyio import SpooledTemporaryFile as anyio_SpooledTemporaryFile a = SpooledTemporaryFile() b = anyio_SpooledTemporaryFile()

I think it defaults to bytes. My IDE is running pyright (strict mode):

Curious, mypy reports the type as SpooledTemporaryFile[builtins.str] while pyright reports it as SpooledTemporaryFile[Unknown].

from tempfile import SpooledTemporaryFile from anyio import SpooledTemporaryFile as AnyIOSpooledTemporaryFile async def main() -> None: with SpooledTemporaryFile() as f1: reveal_type(f1) async with AnyIOSpooledTemporaryFile() as f2: reveal_type(f2)

Gives me:

/home/alex/workspace/anyio/client.py:7:21 - information: Type of "f1" is "SpooledTemporaryFile[bytes]" /home/alex/workspace/anyio/client.py:10:21 - information: Type of "f2" is "SpooledTemporaryFile[Unknown]"

Ahh...I didn't notice before that those TemporaryFile etc. in typeshed were actually functions! That explains a few things. I'll work on fixing the SpooledTemporaryFile implementation tomorrow.

I refactored SpooledTemporaryFile in a way that should satisfy @graingert. However, this caused a lot fo mypy errors that I can't understand.

I'm sorry for bringing this up so late. However, since the official documentation states, The dir, prefix and suffix parameters have the same meaning and defaults as with mkstemp() I assumed that the parameters for mkstemp are of type AnyStr, and I proceeded with that approach.

https://docs.python.org/3/library/tempfile.html#tempfile.mkstemp

Whatever the reason, using AnyStr in both the class type argument AND the initializer arguments makes no sense because the type parameter in the class is supposed to determine whether the input and output type is str or bytes. But the reason for the initializer arguments to be parametrized is because they in part determine file name and you want them to be either str or bytes. Two different, unrelated uses.

Whatever the reason, using AnyStr in both the class type argument AND the initializer arguments makes no sense because the type parameter in the class is supposed to determine whether the input and output type is str or bytes. But the reason for the initializer arguments to be parametrized is because they in part determine file name and you want them to be either str or bytes. Two different, unrelated uses.

I agree with this change

11kkw and others added 2 commits February 11, 2025 01:21

feat: Add async support for temporary file handling. Closes agronholm…

beb8285

…#344

Merge branch 'master' into async-tempfile-support

07730ef

agronholm requested changes Feb 10, 2025

View reviewed changes

11kkw and others added 2 commits February 12, 2025 02:39

Merge branch 'master' into async-tempfile-support

546a462

agronholm reviewed Feb 11, 2025

View reviewed changes

src/anyio/_core/_tempfile.py Outdated Show resolved Hide resolved

agronholm requested a review from graingert February 11, 2025 18:15

graingert reviewed Feb 12, 2025

View reviewed changes

agronholm reviewed Feb 12, 2025

View reviewed changes

src/anyio/_core/_tempfile.py Outdated Show resolved Hide resolved

agronholm requested changes Feb 13, 2025

View reviewed changes

src/anyio/_core/_tempfile.py Outdated Show resolved Hide resolved

src/anyio/_core/_tempfile.py Outdated Show resolved Hide resolved

src/anyio/_core/_tempfile.py Outdated Show resolved Hide resolved

11kkw and others added 5 commits February 15, 2025 01:37

Update src/anyio/_core/_tempfile.py

2e6ea50

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update src/anyio/_core/_tempfile.py

db3c2cd

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Merge branch 'master' into async-tempfile-support

50c498f

Removed redundant py3.14 fixes

b7a9722

agronholm requested changes Feb 15, 2025

View reviewed changes

Kludex mentioned this pull request Feb 16, 2025

Detect blocking calls in coroutines using BlockBuster encode/starlette#2858

Open

3 tasks

11kkw and others added 6 commits February 16, 2025 22:37

Update src/anyio/_core/_tempfile.py

2a1a1cf

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update src/anyio/_core/_tempfile.py

e608cdc

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update tests/test_tempfile.py

4cdadc6

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update tests/test_tempfile.py

bd36d3c

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update tests/test_tempfile.py

86719ec

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update tests/test_tempfile.py

c129057

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

11kkw and others added 5 commits February 16, 2025 22:38

Update tests/test_tempfile.py

6551774

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update tests/test_tempfile.py

f730c72

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update tests/test_tempfile.py

4dc6c71

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

Update tests/test_tempfile.py

bd61572

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

docs: Include TemporaryFile-related classes in API reference

1509c28

agronholm reviewed Feb 16, 2025

View reviewed changes

docs/api.rst Outdated Show resolved Hide resolved

api.rst update

78bb1d1

Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>

graingert reviewed Feb 17, 2025

View reviewed changes

agronholm reviewed Feb 17, 2025

View reviewed changes

Merge branch 'master' into async-tempfile-support

9eaa5c5

Kludex reviewed Feb 22, 2025

View reviewed changes

agronholm added 2 commits February 23, 2025 19:52

Reimplemented SpooledTemporaryFile and improved type annotations

77e7259

Updated tests

058ab8a

		self._async_file = None


		class SpooledTemporaryFile(Generic[AnyStr]):



		@pytest.mark.parametrize("prefix", [b"mkdtemp_", "mkdtemp_"])
		async def test_mkdtemp(prefix: AnyStr) -> None:

	async def test_mkdtemp(prefix: AnyStr) -> None:
	async def test_mkdtemp(prefix: str \| bytes) -> None:

Add async support for temporary file handling. Closes #344 #867

Are you sure you want to change the base?

Add async support for temporary file handling. Closes #344 #867

Conversation

11kkw commented Feb 10, 2025 • edited Loading

Changes

Checklist

Updating the changelog

Additional Notes

agronholm left a comment

Choose a reason for hiding this comment

agronholm commented Feb 11, 2025

agronholm commented Feb 11, 2025

11kkw commented Feb 11, 2025

graingert Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agronholm commented Feb 13, 2025

agronholm commented Feb 13, 2025

agronholm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graingert Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agronholm commented Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

11kkw Feb 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

11kkw commented Feb 10, 2025 •

edited

Loading

graingert Feb 12, 2025 •

edited

Loading

graingert Feb 17, 2025 •

edited

Loading

11kkw Feb 23, 2025 •

edited

Loading