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.

Sign up for GitHub

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

Jump to bottom

pathlib.Path-alike no longer works in 3.9.0 for web.FileResponse #7928

Open
1 task done
kohtala opened this issue Dec 1, 2023 · 4 comments · May be fixed by #7933
Open
1 task done

pathlib.Path-alike no longer works in 3.9.0 for web.FileResponse #7928

kohtala opened this issue Dec 1, 2023 · 4 comments · May be fixed by #7933
Labels
enhancement

Comments

@kohtala
Copy link
Contributor

kohtala commented Dec 1, 2023

Describe the bug

I use web.FileResponse to respond with a file that is not really on the disk. This is done with a Path instance that implements the pathlib.Path interface. Much like the zipfile.Path in the standard library. (zipfile.Path has no stat yet, so it is a no-go anyway.)

With upgrade to 3.9.0, this no longer works, but it fails with file not found exception.

I see the change came in #7111, the backport of #6876. For no apparent reason, the condition to only replace str with pathlib.Path was changed to create pathlib.Path for any argument.

@@ -55,10 +55,7 @@ class FileResponse(StreamResponse):
     ) -> None:
         super().__init__(status=status, reason=reason, headers=headers)

-        if isinstance(path, str):
-            path = pathlib.Path(path)
-
-        self._path = path
+        self._path = pathlib.Path(path)
         self._chunk_size = chunk_size

Python 3.12 is preparing for pathlib.Path to be subclassed more easily (see whatsnew). Whatever reasons there are to subclass, would also be broken by this kind of change.

Since this breaks something that used to work, I report this as a bug. As a new feature it would not require much. Only Path members name, open, stat, with_name and is_file for the value returned by with_name.

To Reproduce

  1. Subclass or implement own pathlib.Path like subclass.
  2. Create instance.
  3. Pass it to web.FileResponse.

Expected behavior

Response created with the stat and file content of the pathlib.Path.

Logs/tracebacks

Traceback (most recent call last):
  File "***/.venv310/lib/python3.10/site-packages/aiohttp/web_protocol.py", line 531, in start
    resp, reset = await task
  File "***/.venv310/lib/python3.10/site-packages/aiohttp/web_protocol.py", line 477, in _handle_request
    reset = await self.finish_response(request, resp, start_time)
  File "***/.venv310/lib/python3.10/site-packages/aiohttp/web_protocol.py", line 630, in finish_response
    await prepare_meth(request)
  File "***/.venv310/lib/python3.10/site-packages/aiohttp/web_fileresponse.py", line 139, in prepare
    st: os.stat_result = await loop.run_in_executor(None, filepath.stat)
  File "/usr/lib/python3.10/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/lib/python3.10/pathlib.py", line 1097, in stat
    return self._accessor.stat(self, follow_symlinks=follow_symlinks)
FileNotFoundError: [Errno 2] No such file or directory: '***'

Python Version

$ python --version
Python 3.10.13

aiohttp Version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.9.1
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author:
Author-email:
License: Apache 2
Location: /home/kohtala/andritz/.venv310/lib/python3.10/site-packages
Requires: aiosignal, async-timeout, attrs, frozenlist, multidict, yarl
Required-by:

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: /home/kohtala/andritz/.venv310/lib/python3.10/site-packages
Requires:
Required-by: aiohttp, yarl

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.9.3
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache-2.0
Location: /home/kohtala/andritz/.venv310/lib/python3.10/site-packages
Requires: idna, multidict
Required-by: aiohttp

OS

$ cat /etc/os-release
NAME="Ubuntu"
VERSION="20.04.6 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.6 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal

Related component

Server

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@kohtala kohtala added the bug label Dec 1, 2023
@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Dec 1, 2023 •
edited
Loading

If you'd like to make a PR to revert that section and include a test that exhibits the problem, we can probably include it.
I'm still not entirely clear why this fails though, I'm curious what the subclass does that breaks when converting to a Path.

kohtala added a commit to okoko/aiohttp that referenced this issue Dec 2, 2023
@kohtala
Support FileResponse from any pathlib.Path compatible object
d2e18a8 
aio-libs#7928
kohtala added a commit to okoko/aiohttp that referenced this issue Dec 2, 2023
@kohtala
Support FileResponse from any pathlib.Path compatible object
f5059ac 
aio-libs#7928
@kohtala kohtala linked a pull request Dec 2, 2023 that will close this issue
Draft
5 tasks
@kohtala
Copy link
Contributor Author

kohtala commented Dec 2, 2023

If you'd like to make a PR to revert that section and include a test that exhibits the problem, we can probably include it.

I created a draft PR. It needs some more thought and work. It just reverts the change, but that is not right solution because it also reverts support for all os.PathLike. It would need to create a pathlib.Path for those that are os.PathLike, but not already compatible with pathlib.Path.

I'll probably add @runtime_checkable to the PathlibPathLike or some hasattr calls to see if the passed object has required pathlib.Path attributes before using it as is.

I'm still not entirely clear why this fails though, I'm curious what the subclass does that breaks when converting to a Path.

The subclass of pathlib.Path replaces stat and open such that it can make the response without the file being on filesystem. The file is created in-memory and having to write it to filesystem just to make a response is pointless. pathlib.Path fails since it can not find it on the filesystem.

kohtala added a commit to okoko/aiohttp that referenced this issue Dec 4, 2023
@kohtala
Support FileResponse from any pathlib.Path compatible object
bd3af50 
aio-libs#7928
kohtala added a commit to okoko/aiohttp that referenced this issue Jan 22, 2024
@kohtala
Support FileResponse from any pathlib.Path compatible object
f07b1e3 
aio-libs#7928
@steverep
Copy link
Member

I use web.FileResponse to respond with a file that is not really on the disk. This is done with a Path instance that implements the pathlib.Path interface.

What's the actual data? Are you opening a file and just want to keep it open? It would not be terribly difficult to allow the constructor to also accept a file object, in particular an instance of BufferedIOBase. The user would just also have to provide the content type since that is guessed from the file extension.

@kohtala
Copy link
Contributor Author

kohtala commented Aug 13, 2024

What's the actual data? Are you opening a file and just want to keep it open? It would not be terribly difficult to allow the constructor to also accept a file object, in particular an instance of BufferedIOBase. The user would just also have to provide the content type since that is guessed from the file extension.

It is not a file at all. It is in-memory generated media data that changes infrequently and has timestamp and content-type from the extension. I like to use the implementation of RFC 7232 and RFC 7233 that is in FileResponse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support FileResponse from any pathlib.Path compatible object okoko/aiohttp
3 participants
@kohtala @Dreamsorcerer @steverep