bito-code-review[bot] commented on code in PR #40860: URL: https://github.com/apache/superset/pull/40860#discussion_r3383425627
########## superset/translations/zh_TW/LC_MESSAGES/messages.po: ########## @@ -4606,6 +4606,9 @@ msgstr "資料庫設定已更新" msgid "Database type does not support file uploads." msgstr "資料庫不支持子查詢" +msgid "Database upload file exceeds the maximum allowed size." +msgstr "" Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Incomplete translation entry</b></div> <div id="fix"> The new translation entry `Database upload file exceeds the maximum allowed size.` has an empty `msgstr`. Users will see English text instead of Chinese. Per adaptive rule [6516], all user-facing text should be properly translated. </div> </div> <small><i>Code Review Run #491760</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them ########## superset/databases/api.py: ########## @@ -1726,6 +1726,7 @@ def upload_metadata(self) -> Response: parameters = UploadFileMetadataPostSchema().load(request_form) except ValidationError as error: return self.response_400(message=error.messages) + UploadCommand.validate_file_size(parameters["file"]) Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Missing exception handler for file size validation</b></div> <div id="fix"> The `DatabaseUploadFileTooLarge` exception raised here inherits from `CommandException` and carries `status = 413`, but the `upload_metadata` endpoint (unlike most other endpoints in this file) lacks the `@handle_api_exception` decorator. Without explicit handling, the app-level fallback handler catches `CommandException` and returns HTTP 500 regardless of the exception's `status` attribute. This means oversized-file requests get a 500 response instead of 413. Wrap this call in a try/except that catches `DatabaseUploadFileTooLarge` and calls `self.response_413(...)` to return the correct status code. </div> <details> <summary> <b>Code suggestion</b> </summary> <blockquote>Check the AI-generated fix before applying</blockquote> <div id="code"> ``` --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -1726,6 +1726,10 @@ class DatabaseRestApi(BaseSupersetModelRestApi): parameters = UploadFileMetadataPostSchema().load(request_form) except ValidationError as error: return self.response_400(message=error.messages) + try: + UploadCommand.validate_file_size(parameters["file"]) + except DatabaseUploadFileTooLarge as error: + return self.response_413(message=error.message) if parameters["type"] == UploadFileType.CSV.value: metadata = CSVReader(parameters).file_metadata(parameters["file"]) elif parameters["type"] == UploadFileType.EXCEL.value: ``` </div> </details> </div> <small><i>Code Review Run #5256f8</i></small> </div><div> <div id="suggestion"> <div id="issue"><b>Missing test for file size validation</b></div> <div id="fix"> The file size validation logic is correctly implemented and will be caught by the `@handle_api_exception` decorator. However, there is no test coverage for this new validation path in the `upload_metadata` endpoint. Add a test that uploads a file exceeding `UPLOAD_MAX_FILE_SIZE_BYTES` and asserts the response is 413 with the expected error structure. </div> </div> <small><i>Code Review Run #491760</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
