fix(load_examples): better fix for load_data #12702
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
craig-rueda
requested changes
Jan 22, 2021
| @@ -125,6 +125,8 @@ def import_dataset( | |||
| def load_data( | |||
| data_uri: str, dataset: SqlaTable, example_database: Database, session: Session | |||
| ) -> None: | |||
| from superset import conf | |||
There was a problem hiding this comment.
Use current_app.config here
| @@ -137,7 +139,7 @@ def load_data( | |||
| df[column_name] = pd.to_datetime(df[column_name]) | |||
|
|
|||
| # reuse session when loading data if possible, to make import atomic | |||
| if example_database.sqlalchemy_uri == get_example_database().sqlalchemy_uri: | |||
| if example_database.sqlalchemy_uri == conf.get("SQLALCHEMY_DATABASE_URI"): | |||
There was a problem hiding this comment.
I think you should check if they're the same OR the examples URI is null, as the loader logic uses the main URI as a fallback
There was a problem hiding this comment.
example_database.sqlalchemy_uri will be set to SQLALCHEMY_DATABASE_URI if SQLALCHEMY_EXAMPLES_URI is not set, so this should catch it, no?
Codecov Report
@@ Coverage Diff @@
## master #12702 +/- ##
==========================================
- Coverage 66.93% 63.85% -3.08%
==========================================
Files 1019 487 -532
Lines 49824 30017 -19807
Branches 4877 0 -4877
==========================================
- Hits 33350 19168 -14182
+ Misses 16351 10849 -5502
+ Partials 123 0 -123
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
craig-rueda
approved these changes
Jan 22, 2021
henryyeh
pushed a commit
to preset-io/superset
that referenced
this pull request
Jan 23, 2021
* fix(load_examples): better fix for load_data * Address changes (cherry picked from commit 9a159b3)
This was referenced Mar 11, 2024
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Revert #12351, while fixing the problem it tried to fix.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TEST PLAN
Ran
superset load-examples, data was loaded into the examples DB and dashboards work.ADDITIONAL INFORMATION