-
Notifications
You must be signed in to change notification settings - Fork 14k
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
fix(data-upload): make to change err message #19430
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19430 +/- ##
=======================================
Coverage 66.59% 66.59%
=======================================
Files 1670 1670
Lines 63884 63884
Branches 6510 6510
=======================================
Hits 42542 42542
Misses 19653 19653
Partials 1689 1689
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -2810,7 +2810,7 @@ def schemas_access_for_file_upload(self) -> FlaskResponse: | |||
get the schema access control settings for file upload in this database | |||
""" | |||
if not request.args.get("db_id"): | |||
return json_error_response("No database is allowed for your csv upload") | |||
return json_error_response("No database is allowed for your file upload") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this is not being used when uploading a CSV as well?
@rusackas do you know if this API is being called from multiple places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This API is used when the csv && excel && columar file is uploaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, and to me this is fine, but since the message now is more generic, I just want to be sure there's no problem with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, cool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine. Approving the PR, but I'll let @yousoph merge it if she agrees, or leave a comment if she doesn't ;)
SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE:
AFTER:
TESTING INSTRUCTIONS
How to reproduce this issue
Precondition: do not enable data upload for database
1, go to columnar file upload
2, observe the error msg in the columnar file upload page
ADDITIONAL INFORMATION