Bugfix malloc upload#106
Open
folke-nordunet wants to merge 7 commits into
Open
Conversation
0c07fd2 to
6aacc76
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves robustness of large-file uploads and job table polling by switching uploads to a disk-backed workflow, making multipart spooling configurable, and adjusting UI behavior to remain responsive while backend processing completes.
Changes:
- Stream uploads to the backend from temporary disk files (instead of reading full file bytes into memory) and clean up temp files after upload attempts.
- Make Starlette multipart spooling size configurable via settings and refine upload dialog status messaging/flow.
- Adjust home page polling to avoid clearing the jobs table during temporary empty responses and tweak initial loading/polling behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| utils/settings.py | Adds a configurable setting for multipart spool max size. |
| utils/common.py | Reworks upload handling to be disk-backed/streamed and updates upload UI flow/status updates. |
| pages/home.py | Updates jobs table polling/initial load to reduce UI churn during backend processing. |
Comments suppressed due to low confidence (1)
utils/common.py:892
post_filecatchesHTTPStatusErrorand callsui.notify, buthandle_upload_with_feedbackalready shows success/failure notifications. This can lead to duplicate error notifications and (sincepost_fileis called from a background task) theui.notifyhere may run outside awith client:context. Consider removing UI side effects frompost_file(return an error/False) and let the caller notify within the client context.
if response.status_code != 200:
raise httpx.HTTPStatusError(
f"Upload failed, status code: {response.status_code}",
request=response.request,
response=response,
)
except httpx.HTTPStatusError as e:
ui.notify(
f"Error when uploading file: {str(e)}", type="negative", position="top"
)
return False
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| file_items.append((file_name, temp_path)) | ||
| except Exception: | ||
| if os.path.exists(temp_path): | ||
| os.remove(temp_path) |
| table.update_rows(await jobs_get(), clear_selection=False) | ||
| rows = await jobs_get() | ||
| with client: | ||
| if rows or not table.rows: |
Comment on lines
+200
to
+207
| Avoid clearing the existing table during temporary backend/API failures. | ||
| This can happen while large uploads are being stored and encrypted. | ||
| """ | ||
| rows = await jobs_get() | ||
|
|
||
| if not rows and table.rows: | ||
| return | ||
|
|
Comment on lines
+223
to
+225
| rows = await jobs_get() | ||
| table.rows = rows | ||
| table.selection = "multiple" if rows else "none" |
Collaborator
|
Thanks. I've been trying to keep everything in memory in order to not risk leaving any files on disk with potential sensitive data. Can you try the feature_upload_memory_bug branch and see if that one solves the problem for you? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves large file upload handling and makes the upload UI more reliable while files are being stored and processed by the backend.
The main change is that uploaded files are no longer read fully into memory before being sent to the backend. Instead, uploads are saved temporarily to disk and streamed from there. This reduces memory pressure and avoids malloc/RAM issues when uploading large files.
Main changes
Why
Large uploads could previously cause excessive memory usage because files were handled as in-memory byte objects. This PR changes the upload flow to be disk-backed, making uploads more robust for large files and improving the user experience during longer upload/storage operations.