-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
chore: consolidate datasource import logic #11533
chore: consolidate datasource import logic #11533
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11533 +/- ##
==========================================
- Coverage 62.25% 55.42% -6.84%
==========================================
Files 874 406 -468
Lines 42325 14344 -27981
Branches 3972 3691 -281
==========================================
- Hits 26350 7950 -18400
+ Misses 15795 6231 -9564
+ Partials 180 163 -17
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -324,7 +325,7 @@ def run(self) -> None: | |||
self.validate() | |||
|
|||
for file_name, content in self.contents.items(): | |||
logger.info(f"Importing dashboard from file {file_name}") | |||
logger.info("Importing dashboard from file %s", file_name) |
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 keep fstrings here?
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.
@hughhhh when logging pylint actually recommends string interpolation, since it's faster (the string is interpolated only if the log level is being recorded) and safer (if the string interpolation fails something is still logged). I changed it here because pylint was complaining.
40b607d
to
6ea9908
Compare
* Consolidate dash import logic * WIP * Add license * Fix lint * Retrigger tests * Fix lint
Note: this is a stacked PR, dependent on #11529.
SUMMARY
Currently the import functionality of Superset is split between the
ImportExportMixin
and theimport_obj
method in some of the models.This PR finalizes the process of getting rid of the
import_obj
method, started in #11529, consolidating all the import functionality in a single place to make it easier to preserve backwards compatibility once we implement #11167.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TEST PLAN
Updated unit tests.
ADDITIONAL INFORMATION