Bump python sdk version #827
Open
+224
−308
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
… config file (#724) Signed-off-by: Artur Peedimaa <[email protected]> Co-authored-by: Artur Peedimaa <[email protected]> Co-authored-by: Ben Cassell <[email protected]>
Signed-off-by: Dmitry Volodin <[email protected]> Co-authored-by: Ben Cassell <[email protected]>
Co-authored-by: Antoine <[email protected]>
Signed-off-by: Roy Dobbe [email protected] Co-authored-by: Roy Dobbe <[email protected]>
Signed-off-by: Kyle Valade <[email protected]> Co-authored-by: Kyle Valade <[email protected]> Co-authored-by: Ben Cassell <[email protected]>
…s into bump_python_sdk_version
eric-wang-1990
requested review from
andrefurlan-db,
benc-db and
rcypher-databricks
as code owners
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
temporarily deployed
to
azure-prod
GitHub Actions
Inactive
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Failure
jackyhu-db
reviewed
Nov 15, 2024
| self._lock.acquire() | ||
| try: | ||
| assert self._credentials_manager is not None, "Credentials manager is not set." | ||
| return self._credentials_manager |
There was a problem hiding this comment.
is lock still required?
jackyhu-db
reviewed
Nov 15, 2024
| auth_type = ( | ||
| "external-browser" if not self.client_secret | ||
| # if the client_secret starts with "dose" then it's likely using oauth-m2m | ||
| else "oauth-m2m" if self.client_secret.startswith("dose") |
There was a problem hiding this comment.
can it wrapped in a method like is_databricks_m2m? And this checks still a little bit tricky, do we really want to infer it from secret format? Can it be explicitly specified by the config to decide whether azure or databricks m2m should be used?
There was a problem hiding this comment.
+1 to what Jacky said. In design review we decided to not depend on secret format, but instead to a.) reference if a config flag is set to tell us to use AAD, and b.) if we fail with Databricks OAuth but succeed with AAD, emit a warning telling them to set the config flag.
|
Thanks @eric-wang-1990 for updating this. Can your rename the PR title to make it more readable? |
|
Hi Eric, since the last time we were looking at this PR, 1.9 has become the target in main. So please rebase and target main with PR. |
benc-db
requested changes
Nov 15, 2024
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Error
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Error
eric-wang-1990
had a problem deploying
to
azure-prod
GitHub Actions
Error
… bump_python_sdk_version
eric-wang-1990
requested a deployment
to
azure-prod
GitHub Actions
In progress
eric-wang-1990
requested a deployment
to
azure-prod
GitHub Actions
In progress
eric-wang-1990
requested a deployment
to
azure-prod
GitHub Actions
In progress
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.
Upgrade python sdk version from 0.17.0 to 0.36.0
Create DatabricksCredentialManager to managed credentials, under the hood use Config from databricks.sdk.core to manage the auth part which comes from the Python SDK.
For dbt we support 4 different auth mode:
For oauth-m2m and azure-client-secret we set a preferred auth sequence based on if the clientSecret starts with "dose", so we can reduce the number of false trying as much as possible.
Remove the set/get sharedPassword part, since the python sdk already handles that, but with one defect which I already raise a PR against: databricks/databricks-sdk-py#823
With this change one thing that will no longer work is if customer are using non-databricks OAuth client for U2M case it will not work, since the python sdk does not support modify redirectUrl/scopes.
Description
Checklist
CHANGELOG.mdand added information about my change to the "dbt-databricks next" section.