-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Passing UploadFile objects into a StreamingResponse closes it in v0.106.0 but not v0.105.0 #10857
Comments
Is there any update on this issue? I have the same problem , just migrated from async def upload_csv_to_gcloud_blob_storage(
self, file: UploadFile, bucket_name: str, blob_storage_path:str
):
"""write the data to a bucket with"""
storage_client = storage.Client()
csv_bucket = storage_client.bucket(bucket_name)
blob = csv_bucket.blob(blob_storage_path)
try:
with blob.open("wb") as buffer:
shutil.copyfileobj(file.file, buffer)
finally:
file.file.close()
@app.post( "/csv_tables" )
async def upload_csv(
csv_file: Annotated[UploadFile, File()],
background_tasks: BackgroundTasks,
):
background_tasks.add_task(
upload_csv_to_gcloud_blob_storage,
csv_file=csv_file,
bucket_name="csv",
blob_storage_path="csv_path",
)
return "OK" |
We ran into this issue. Calling |
I'm having similar issue with a tempfile I'm yielding....
When I try to return the file in a FileResponse I hit: |
It seems to be, removing this line prevent the file in the body from being closed: Line 231 in 958425a
It was introduced in the only commit changing the code between versions 105 and 106: Unfortunately the |
I went through the Line 294 in 958425a
and then closes the file. But in case the handler is asynchronous, like a background task: @app.post(...)
async def foo():
background_tasks.add_task(process_recording_file, file=file, ...) it closes the file before the task can actually run. So I created a deep copy like this: -background_tasks.add_task(process_recording_file, file=file, ...)
+background_tasks.add_task(process_recording_file, file=copy.deepcopy(file), ...) and I close the file myself in the function: async def process_recording_file(file: UploadFile...):
try:
...
finally:
await file.close() which fixes the issue even with the newest version of FastAPI. It is probably not very efficient given that the file is few MB in my case. So maybe a better question would be, what is the expected usage of UploadFile combined with BackgroundTasks? https://fastapi.tiangolo.com/tutorial/background-tasks/ |
Hi, @msehnout Interesting thinking, but I don't think it's a backstage task. Look at this.:encode/starlette#2176 (comment) |
How is my comment related? |
I just want to express that the idea of @ msehnout may be backward incompatible because he mentioned background tasks,it seems to have nothing to do with your comments? |
Related discussion: #10850 I am assuming |
I'm not sure the problem I'm experiencing is actually an issue or a bad design choice on my side, so I created a discussion instead: #10936 |
Any updates on this? deepcopying isn't an option for me. |
Looking forward to an update. copy.deepcopy not work for me. I have no choice but to downgrade fastapi |
I'm also hitting this. Need to perform some long running processing steps on uploaded files and it's no longer possible with BackgroundTasks due to the file being closed when returning the response. So this promise from the FastAPI docs on BackgroundTasks is now broken.
|
In case it helps others, for now I'm working around this issue by adding my own from fastapi import UploadFile as _UploadFile
class UploadFile(_UploadFile):
"""Patches `fastapi.UploadFile` due to buffer close issue.
See the related github issue:
https://github.com/tiangolo/fastapi/issues/10857
"""
def __init__(self, upload_file: _UploadFile) -> None:
"""Wraps and mutates input `fastapi.UploadFile`.
Swaps `close` method on the input instance so it's a no-op when called
by the framework. Adds `close` method of input as `_close` here, to be
called later with overridden `close` method.
"""
self.filename = upload_file.filename
self.file = upload_file.file
self.size = upload_file.size
self.headers = upload_file.headers
_close = upload_file.close
setattr(upload_file, "close", self._close)
setattr(self, "_close", _close)
async def _close(self) -> None:
pass
async def close(self) -> None: # noqa: D102
await self._close() |
Also running into this, any plans for a timely fix? |
We're also hitting this and are stuck with FastAPI 0.105.0 on multiple projects until this is fixed. |
Discussed in #10856
Originally posted by adrwz December 28, 2023
First Check
Commit to Help
Example Code
Description
Hitting this endpoint with an uploaded file on v0.105.0 will print:
Hitting this endpoint with an uploaded file on v0.106.0 will print:
Unfortunately this means I'm unable to upload files in a StreamingResponse with fastapi>=0.106.0
Operating System
macOS
Operating System Details
No response
FastAPI Version
0.105.0
Pydantic Version
2.5.3
Python Version
3.11.4
Additional Context
No response
The text was updated successfully, but these errors were encountered: