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

feat: add site setting upload_max_body_size #497

Merged
merged 8 commits into from
Dec 16, 2024

Conversation

sszgwdk
Copy link
Collaborator

@sszgwdk sszgwdk commented Dec 13, 2024

fix bug #382

Copy link

vercel bot commented Dec 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tidb-ai-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 16, 2024 1:51am
tidb-ai-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 16, 2024 1:51am

Copy link

vercel bot commented Dec 13, 2024

@sszgwdk is attempting to deploy a commit to the pingcap Team on Vercel.

A member of the Team first needs to authorize it.

backend/app/site_settings/default_settings.yml Outdated Show resolved Hide resolved
Comment on lines 37 to 39
if SiteSetting.setting_exists("upload_max_body_size"):
sys_upload_max_body_size = SiteSetting.get_setting("upload_max_body_size")
if file.size > sys_upload_max_body_size:
Copy link
Member

Choose a reason for hiding this comment

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

As the __getattr__ method has been overrided,

def __getattr__(self, name: str) -> SettingType:

You can just:

Suggested change
if SiteSetting.setting_exists("upload_max_body_size"):
sys_upload_max_body_size = SiteSetting.get_setting("upload_max_body_size")
if file.size > sys_upload_max_body_size:
sys_upload_max_body_size = SiteSetting.upload_max_body_size
if sys_upload_max_body_size:
if file.size > sys_upload_max_body_size:


upload:
upload_max_body_size:
default: 10485760
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
default: 10485760
default: 10485760 # 10MB

if file.size > sys_upload_max_body_size:
raise HTTPException(
status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE,
detail=f"File size({file.size}) exceeds maximum allowed size({sys_upload_max_body_size})",
Copy link
Member

Choose a reason for hiding this comment

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

Can me make the file size humam-readble?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@Mini256 Mini256 requested a review from wd0517 December 13, 2024 10:55
Copy link
Member

@Mini256 Mini256 left a comment

Choose a reason for hiding this comment

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

rest LGTM

backend/app/site_settings/default_settings.yml Outdated Show resolved Hide resolved
backend/app/site_settings/default_settings.yml Outdated Show resolved Hide resolved
backend/app/site_settings/default_settings.yml Outdated Show resolved Hide resolved
backend/app/api/admin_routes/upload.py Outdated Show resolved Hide resolved
@sykp241095 sykp241095 merged commit 4146511 into pingcap:main Dec 16, 2024
8 checks passed
@sszgwdk sszgwdk deleted the upload_file_max_body_size branch December 19, 2024 02:30
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.

4 participants