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

Set a default size when using SpooledTemporaryFile #1359

Merged
merged 1 commit into from Apr 20, 2024

Conversation

RealOrangeOne
Copy link
Contributor

Leaving 0 as the default means the file never gets rolled over to disk, resulting in higher memory usage than expected (or needed).

S3 and GCS both used 0 for their default value. Now, they use FILE_UPLOAD_MAX_MEMORY_SIZE (2.5MB by default). This is significantly better than 0, but still allows configuration as needed.

Azure already set a default of 2MB, which is better, but now brought in line with S3 and GCS. Dropbox previously didn't allow configuration, so an additional setting was added.

Leaving `0` as the default means the file never gets rolled over to disk, resulting in higher memory usage than expected (or needed).

S3 and GCS both used `0` for their default value. Now, they use `FILE_UPLOAD_MAX_MEMORY_SIZE` (2.5MB by default). This is significantly better than `0`, but still allows configuration as needed.

Azure already set a default of 2MB, which is better, but now brought in line with S3 and GCS
@jschneier
Copy link
Owner

This is a breaking change but I think a very reasonable one to follow upstream, thanks.

@jschneier
Copy link
Owner

Everything should go green if you rebase now

@RealOrangeOne
Copy link
Contributor Author

@jschneier over to you 😄

@jschneier
Copy link
Owner

Thanks this is a breaking change for people with read only file systems most likely so will need to put it as such in the notes.

@jschneier jschneier merged commit 5111199 into jschneier:master Apr 20, 2024
19 checks passed
jschneier added a commit that referenced this pull request Apr 21, 2024
jschneier added a commit that referenced this pull request Apr 21, 2024
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

Successfully merging this pull request may close these issues.

None yet

2 participants