-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
[fix] SQL query source #9173
[fix] SQL query source #9173
Conversation
6555b62
to
014506d
Compare
@@ -314,6 +314,12 @@ def get_sqla_engine( | |||
params.update(self.get_encrypted_extra()) | |||
|
|||
if DB_CONNECTION_MUTATOR: | |||
if not source and request and request.referrer: |
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.
This is similar logic to what was defined in get_df
however it's been pushed further up the stack. Note we only try to mutate the source if it isn't explicitly defined.
Codecov Report
@@ Coverage Diff @@
## master #9173 +/- ##
==========================================
- Coverage 59.16% 59.13% -0.04%
==========================================
Files 372 372
Lines 11935 11938 +3
Branches 2927 2925 -2
==========================================
- Hits 7061 7059 -2
- Misses 4694 4699 +5
Partials 180 180
Continue to review full report at Codecov.
|
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, one nit
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 👍
014506d
to
5a4cb9c
Compare
(cherry picked from commit 1415706)
(cherry picked from commit 1415706)
CATEGORY
Choose one
SUMMARY
This PR makes a couple of updates to the encoding of the query source for SQL queries:
get_df
toget_sqla_engine
.The reason for the later was there were a number of places where
get_sqla_engine
were being called without a source and it felt that this would be a better place to add the fallback logic if the source wasn't explicitly defined.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
CI and tested locally.
ADDITIONAL INFORMATION
REVIEWERS
to: @etr2460 @villebro