refactor: external metadata fetch API#16193
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16193 +/- ##
==========================================
- Coverage 76.75% 76.71% -0.05%
==========================================
Files 996 997 +1
Lines 53054 53073 +19
Branches 6760 6740 -20
==========================================
- Hits 40721 40714 -7
- Misses 12107 12130 +23
- Partials 226 229 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
villebro
left a comment
villebro
left a comment
There was a problem hiding this comment.
Looks great - one minor question before pressing approve
| Object.keys(params).map(key => | ||
| params[key] === undefined ? null : params[key], | ||
| ), |
There was a problem hiding this comment.
Don't we need to encode the key/value pairs (Object.entries), not just the values (Object.keys)? (honest question, I haven't done a lot of this rison API stuff)
There was a problem hiding this comment.
this is to code entire params field, just change undefined to null, because prison can't encode undefined value
villebro
left a comment
villebro
left a comment
There was a problem hiding this comment.
LGTM, and special thanks for the comprehensive tests!
…gies * upstream/master: (64 commits) check roles before fetching reports (#16260) chore: upgrade mypy and add type guards (#16227) fix: pivot columns with ints for name (#16259) chore(pylint): Bump Pylint to 2.9.6 (#16146) fix examples tab for dashboard (#16253) chore: bump superset-ui packages to 0.17.84 (#16251) chore: Shows the dataset description in the gallery dropdown (#16200) fix(Dashboard): Omnibar dropdown visibility and keyboard commands (#16168) chore: bump py version for integration test (#16213) fix: skip perms on query context update (#16250) refactor: external metadata fetch API (#16193) feat(dao): admin can remove self from object owners (#15149) fix(dashboard): cross filter chart highlight when filters badge icon clicked (#16233) fix: validate_parameters and query (#16241) fix: Remove Advanced Analytics tag for 2 charts (#16240) Revert "feat: Changing Dataset names (#16199)" (#16235) feat: Allow users to connect via legacy SQLA form (#16201) fix: remove encryption from db params (#16214) fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel (#16060) Show/hide tooltips (#16192) ... # Conflicts: # superset/tasks/caching/cache_strategy.py
| from typing_extensions import TypedDict | ||
|
|
||
|
|
||
| class ExternalMetadataParams(TypedDict): |
There was a problem hiding this comment.
@villebro @zhaoyongjie I wonder if we should be using marshmallow_dataclass instead as it adhere's to the DRY principle and simplifies the code. I've used it for an internal project and I'm definitely a fan.
There was a problem hiding this comment.
Thanks for the mention. It's a more elegant way, let me try to use marshmallow_dataclass to refactor this class.
* refactor: external metadata api * fix comments * fix ut * fix fe lint * fix UT * fix UT (cherry picked from commit 6cd15d5)
* refactor: external metadata api * fix comments * fix ut * fix fe lint * fix UT * fix UT
* refactor: external metadata api * fix comments * fix ut * fix fe lint * fix UT * fix UT
mistercrunch
added
🍒 1.3.1
5 participants
SUMMARY
closes: #16172
When special characters("/") are encountered, the API
/datasource/external_metadata_by_name/<datasouce_type>/<database_name>/<schema_name>/<table_name>cannot be called.This PR refactor this API to
/datasource/external_metadata_by_name/?q=<rison string>BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After
virtual_dataset_sync.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION