[PECO-1387] Fix: mv/st network request authentication is being overridden by local .netrc file #555
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
added 19 commits
May 19, 2023 12:10
Update changelog. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Don't use dbt-spark as a default. This is a no-op because this is always overridden anyway. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
setting the BearerAuth on the session rather than per-request Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
refactor to push the .auth into requests session. It's not required in DBContext anymore. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
.netrc fix. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
This was leftover from moving the authentication class into connections.py in 4f56a5c Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
This fixes a merge conflict in the changelog Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
…override Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
susodapop
requested review from
andrefurlan-db,
benc-db and
rcypher-databricks
as code owners
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
susodapop
had a problem deploying
to
azure-prod
GitHub Actions
Failure
susodapop
had a problem deploying
to
azure-prod
GitHub Actions
Failure
susodapop
had a problem deploying
to
azure-prod
GitHub Actions
Failure
added 2 commits
January 17, 2024 18:10
it around between method calls. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
added 2 commits
January 18, 2024 15:07
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
susodapop
commented
Jan 18, 2024
| pipeline = _get_pipeline_state(host, headers, pipeline_id) | ||
| session = Session() | ||
| session.auth = BearerAuth(headers) | ||
| session.headers = {"User-Agent": self._user_agent} |
There was a problem hiding this comment.
I set the headers on the Session now so we don't need to pass headers to _get_pipeline_state and _get_update_error_message etc.
susodapop
commented
Jan 18, 2024
| @@ -606,7 +611,7 @@ def pollRefreshPipeline( | |||
| # should we do exponential backoff? | |||
| time.sleep(polling_interval) | |||
|
|
|||
| pipeline = _get_pipeline_state(host, headers, pipeline_id) | |||
| pipeline = _get_pipeline_state(session, host, pipeline_id) | |||
There was a problem hiding this comment.
Here you can see I replace headers with session.
benc-db
approved these changes
Jan 18, 2024
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.
Description
This pull request fixes an issue in the materialized view / streaming table code path where a local
.netrcfile in a user's home directory could override the authentication headers used for polling the MV/ST status. The fix is the same as the one in #338 and uses the sameBearerAuthclass.The fix is to use a
requests.Sessionwith its.authproperty set to an instance of our customBearerAuthclass and then replace every call torequests.get(...)with a call toself.session.get(...). For simplicity, I also set theUser-Agentheader at theSessionlevel so that we don't need to pass aheadersdict around between methods.To test this, I ran the
TestMaterializedViewintegration test with a bad.netrcfile in my home directory. Before this change, the test failed with this error:This indicates an authentication error.
After the change in this pull request, the materialized view / streaming table test still fails (because the test is broken - which I confirmed with @rcypher-databricks ) but it doesn't fail because of an authentication issue. This tells me that the fix is working as it should.
As for a direct test of this behaviour, we don't have a clean way to add a bad
.netrcto our cloud testing environment so a manual test like I describe above is the only choice :/Checklist
CHANGELOG.mdand added information about my change to the "dbt-databricks next" section.