-
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
feat: deprecate /superset/extra_table_metadata migrate to api v1 #19921
feat: deprecate /superset/extra_table_metadata migrate to api v1 #19921
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19921 +/- ##
===========================================
- Coverage 66.52% 53.97% -12.56%
===========================================
Files 1714 1714
Lines 65052 65099 +47
Branches 6722 6725 +3
===========================================
- Hits 43279 35139 -8140
- Misses 20061 28248 +8187
Partials 1712 1712
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -775,10 +775,30 @@ def test_get_invalid_table_table_metadata(self): | |||
Database API: Test get invalid table from table metadata | |||
""" | |||
example_db = get_example_database() | |||
uri = f"api/v1/database/{example_db.id}/wrong_table/null/" | |||
uri = f"api/v1/database/{example_db.id}/table/wrong_table/null/" |
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.
Nice catch!
@@ -415,10 +415,10 @@ describe('async actions', () => { | |||
fetchMock.delete(updateTableSchemaEndpoint, {}); | |||
fetchMock.post(updateTableSchemaEndpoint, JSON.stringify({ id: 1 })); | |||
|
|||
const getTableMetadataEndpoint = 'glob:*/api/v1/database/*'; | |||
const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/'; |
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.
isn't a single asterisk here sufficient?
const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/'; | |
const getTableMetadataEndpoint = 'glob:*/api/v1/database/*/table/*/*/'; |
fetchMock.get(getTableMetadataEndpoint, {}); | ||
const getExtraTableMetadataEndpoint = | ||
'glob:*/superset/extra_table_metadata/*'; | ||
'glob:**/api/v1/database/*/table_extra/*/*/'; |
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.
Same here
'glob:**/api/v1/database/*/table_extra/*/*/'; | |
'glob:*/api/v1/database/*/table_extra/*/*/'; |
…che#19921) * feat: deprecate /superset/extra_table_metadata migrate to api v1 * use can_read to table_extra_metadata * troubleshoot sqlite * fix test * fix test * fix test * fix frontend test on sqllab
…che#19921) * feat: deprecate /superset/extra_table_metadata migrate to api v1 * use can_read to table_extra_metadata * troubleshoot sqlite * fix test * fix test * fix test * fix frontend test on sqllab
…che#19921) * feat: deprecate /superset/extra_table_metadata migrate to api v1 * use can_read to table_extra_metadata * troubleshoot sqlite * fix test * fix test * fix test * fix frontend test on sqllab
…che#19921) * feat: deprecate /superset/extra_table_metadata migrate to api v1 * use can_read to table_extra_metadata * troubleshoot sqlite * fix test * fix test * fix test * fix frontend test on sqllab
SUMMARY
Following plan to gradually deprecate "REST" endpoints on /superset namespace.
Deprecates
/superset/extra_table_metadata/<int:database_id>/<table_name>/<schema>/
. Proposed removal on Superset 3.0New endpoint
/api/v1/database/<int:database_id>/table_extra/<table_name>/<schema_name>/
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION