Skip to content

Conversation

khadrawy
Copy link

@khadrawy khadrawy commented Oct 6, 2024

Summary

Limiting the size of file being uploaded in case the storage is limited or some business logic can't handle certain size.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@khadrawy khadrawy force-pushed the feat/upload-file-max-size branch from 2d62595 to bc3b9c3 Compare October 6, 2024 04:24
@khadrawy khadrawy force-pushed the feat/upload-file-max-size branch from bc3b9c3 to d44aa98 Compare October 6, 2024 04:46
@khadrawy khadrawy marked this pull request as ready for review November 15, 2024 06:33
@Kludex
Copy link
Owner

Kludex commented Dec 3, 2024

If we instead make the max_part_size configurable, would that already be good enough here?

@khadrawy
Copy link
Author

khadrawy commented Dec 3, 2024

If we instead make the max_part_size configurable, would that already be good enough here?

But from what I see here is that this is only applied to non file parts.

@Kludex
Copy link
Owner

Kludex commented Dec 3, 2024

If we instead make the max_part_size configurable, would that already be good enough here?

But from what I see here is that this is only applied to non file parts.

Hmmm... Does it make sense to be only on non file parts tho? 🤔

@khadrawy
Copy link
Author

khadrawy commented Dec 3, 2024

@Kludex I think we need different limits for parts vs files. As files are spooled, we could allow larger size but for parts we want to have a stricter limit as it is stored in memory.
I can re-work the PR to make it configurable for both.

@khadrawy
Copy link
Author

@Kludex what do you think about making all of limits configurable? https://github.com/encode/starlette/pull/2798/files
I see that you are going to rename the max_file_size here #2780 I can adjust naming if we can go forward with making all of the size limits configurable.

@khadrawy khadrawy mentioned this pull request Dec 15, 2024
3 tasks
@Kludex
Copy link
Owner

Kludex commented Feb 22, 2025

We can make the limits configurable. We just need to adjust the naming, yep.

Let's start by making the spool one configurable, and then we add 1 by one. 🙏

Can you help?

max_files: int | float = 1000,
max_fields: int | float = 1000,
max_part_size: int = 1024 * 1024,
max_file_spool_size: int = 1024 * 1024,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
max_file_spool_size: int = 1024 * 1024,
max_spool_size: int = 1024 * 1024,

@khadrawy khadrawy force-pushed the feat/upload-file-max-size branch from 98dc855 to 108bf94 Compare February 23, 2025 15:40
@khadrawy khadrawy force-pushed the feat/upload-file-max-size branch from 108bf94 to 5946445 Compare February 23, 2025 15:43
@khadrawy khadrawy requested a review from Kludex February 23, 2025 16:04
@khadrawy khadrawy mentioned this pull request Feb 23, 2025
3 tasks
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.

2 participants