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: validate DB-specific parameters #15155
fix: validate DB-specific parameters #15155
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15155 +/- ##
==========================================
- Coverage 77.54% 77.53% -0.01%
==========================================
Files 967 967
Lines 49752 49783 +31
Branches 6351 6351
==========================================
+ Hits 38578 38598 +20
- Misses 10972 10983 +11
Partials 202 202
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
assert rv.status_code == 200 | ||
assert response == {"message": "OK"} | ||
|
||
def test_validate_parameters_invalid_port(self): |
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.
Can we add validation for the range of ports allowed?
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.
Sure, Arash is adding that on a separate PR.
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.
* fix: validate DB-specific parameters * Fix lint * Update test * Fix lint/test * Fix lint * Update superset/databases/api.py
* fix: validate DB-specific parameters * Fix lint * Update test * Fix lint/test * Fix lint * Update superset/databases/api.py
* fix: validate DB-specific parameters * Fix lint * Update test * Fix lint/test * Fix lint * Update superset/databases/api.py
SUMMARY
The
validate_parameters
endpoint was not performing the DB-specific Marshmallow validation (based onparameters_schema
), but only the validation from thevalidate_parameters
schema in the DB engine spec (which does more deep validation like checking if the host is resolvable and the port is open, but doesn't validate the schema).I changed the
DatabaseValidateParametersSchema
schema validation to also validate the DB-specific parameters, and changed the API to return the error messages expected by the frontend for theonBlur
validation (with aninvalid
key in theextra
).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
Added unit tests.
ADDITIONAL INFORMATION