-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Prevent 'main' database connection creation #8038
Conversation
6d8b8fd
to
f3ef531
Compare
a591f70
to
60fe1f5
Compare
60fe1f5
to
4585492
Compare
4585492
to
ee0614f
Compare
Codecov Report
@@ Coverage Diff @@
## master #8038 +/- ##
==========================================
+ Coverage 66.06% 73.67% +7.6%
==========================================
Files 479 115 -364
Lines 22930 12036 -10894
Branches 2524 0 -2524
==========================================
- Hits 15148 8867 -6281
+ Misses 7648 3169 -4479
+ Partials 134 0 -134
Continue to review full report at Codecov.
|
Will review shortly |
data = self.get_json_resp("/superset/search_queries?database_id=1") | ||
data = self.get_json_resp( | ||
f"/superset/search_queries?database_id={examples_dbid}" | ||
) |
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.
👍
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.
LGTM, however didn't test locally, so if testing help is needed let me know. I'm personally a big fan of having the main
db automatically available, but that may be because I'm just used to the status quo. But perhaps a note in the changelog would be appropriate to inform users that it's no longer created automatically.
if your |
CATEGORY
Choose one
SUMMARY
In #7773, we created a new connection model instance to load and connect to examples. This PR build upon this PR and removes the need for a
main
database connection to the metadata database. Note that thismain
connection database still get created for unit testing purposes, but that should probably get deprecated in favor of leveraging theexamples
database for unit tests.TEST PLAN
Have the current unit test suite succeed (lots of existing tests had to be altered)