Skip to content

Reduce redundant warning messages in azure-cli credential provider#410

Merged
mgyucht merged 1 commit intomainfrom
reduce-azure-cli-extra-logging
Oct 23, 2023
Merged

Reduce redundant warning messages in azure-cli credential provider#410
mgyucht merged 1 commit intomainfrom
reduce-azure-cli-extra-logging

Conversation

@mgyucht
Copy link
Copy Markdown
Contributor

@mgyucht mgyucht commented Oct 18, 2023

Changes

This warning is a bit noisy: in the happy path, we print it multiple times, and even if we don't use Azure CLI auth, we may print it anyways. Also, it will be printed always when using account-level authentication.

This PR reduces the noise for this warning by only printing it when we are certainly going to use Azure CLI auth, authenticating to a workspace, and can't get the subscription ID (e.g. because it is not configured).

Tests

  • Ran locally: with accounts client, no warning is printed, and with workspace client, only one warning is printed.

@mgyucht mgyucht requested a review from hectorcast-db October 18, 2023 11:51
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 18, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Files Coverage Δ
databricks/sdk/core.py 81.32% <83.33%> (+0.83%) ⬆️

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@hectorcast-db
Copy link
Copy Markdown
Contributor

Related PR (which will have a conflict with this)
https://github.com/databricks/databricks-sdk-py/pull/409/files

I was trying to remember which stage and credentials I used in my testing. Since you just tested it, can you confirm that this doesn't happen in production?

@mgyucht
Copy link
Copy Markdown
Contributor Author

mgyucht commented Oct 18, 2023

@hectorcast-db I saw that PR which inspired me to do this one instead. After this change, this warning message will still be printed in production for users who don't specify the workspace resource ID. It can work if the workspace is in the same tenant as their current subscription, but for users who use multiple tenants, it can still be an issue, so I think the warning is well-placed to provide guidance for these otherwise inscrutable errors.

@mgyucht mgyucht added this pull request to the merge queue Oct 23, 2023
Merged via the queue into main with commit f29ad4a Oct 23, 2023
@mgyucht mgyucht deleted the reduce-azure-cli-extra-logging branch October 23, 2023 14:19
mgyucht added a commit that referenced this pull request Oct 24, 2023
* Retry on all 429 and 503, even when missing Retry-After header ([#402](#402)).
* Add support for tokenless authentication for GitHub Actions configured with OpenID Connect with Azure User Managed Identity (or Service Principal) ([#385](#385)).
* Reduce redundant warning messages in azure-cli credential provider ([#410](#410)).

API Changes:

 * Added `attributes`, `count`, `excluded_attributes`, `filter`, `sort_by`, `sort_order`, and `start_index` fields for `databricks.sdk.service.iam.GetAccountUserRequest` and `databricks.sdk.service.iam.GetUserRequest`.
 * Added `schemas` field for `databricks.sdk.service.iam.Group`, `databricks.sdk.service.iam.ListGroupsResponse`, `databricks.sdk.service.iam.ListServicePrincipalResponse`, `databricks.sdk.service.iam.ListUsersResponse`, `databricks.sdk.service.iam.ServicePrincipal`, and `databricks.sdk.service.iam.User`.
 * Added `databricks.sdk.service.iam.GetSortOrder`, `databricks.sdk.service.iam.GroupSchema`, `databricks.sdk.service.iam.ListResponseSchema`, `databricks.sdk.service.iam.ServicePrincipalSchema`, and `databricks.sdk.service.iam.UserSchema` dataclasses.
 * Added `webhook_notifications` field for `databricks.sdk.service.jobs.SubmitTask`.
 * Added [w.apps](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/apps.html) workspace-level service and related dataclasses
 * Added [a.account_network_policy](https://databricks-sdk-py.readthedocs.io/en/latest/account/account_network_policy.html) account-level service and related dataclasses.

OpenAPI SHA: 5903bb39137fd76ac384b2044e425f9c56840e00, Date: 2023-10-23
@mgyucht mgyucht mentioned this pull request Oct 24, 2023
github-merge-queue bot pushed a commit that referenced this pull request Oct 24, 2023
* Retry on all 429 and 503, even when missing Retry-After header
([#402](#402)).
* Add support for tokenless authentication for GitHub Actions configured
with OpenID Connect with Azure User Managed Identity (or Service
Principal)
([#385](#385)).
* Reduce redundant warning messages in azure-cli credential provider
([#410](#410)).

API Changes:

* Added `attributes`, `count`, `excluded_attributes`, `filter`,
`sort_by`, `sort_order`, and `start_index` fields for
`databricks.sdk.service.iam.GetAccountUserRequest` and
`databricks.sdk.service.iam.GetUserRequest`.
* Added `schemas` field for `databricks.sdk.service.iam.Group`,
`databricks.sdk.service.iam.ListGroupsResponse`,
`databricks.sdk.service.iam.ListServicePrincipalResponse`,
`databricks.sdk.service.iam.ListUsersResponse`,
`databricks.sdk.service.iam.ServicePrincipal`, and
`databricks.sdk.service.iam.User`.
* Added `databricks.sdk.service.iam.GetSortOrder`,
`databricks.sdk.service.iam.GroupSchema`,
`databricks.sdk.service.iam.ListResponseSchema`,
`databricks.sdk.service.iam.ServicePrincipalSchema`, and
`databricks.sdk.service.iam.UserSchema` dataclasses.
* Added `webhook_notifications` field for
`databricks.sdk.service.jobs.SubmitTask`.
* Added
[w.apps](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/apps.html)
workspace-level service and related dataclasses
* Added
[a.account_network_policy](https://databricks-sdk-py.readthedocs.io/en/latest/account/account_network_policy.html)
account-level service and related dataclasses.

OpenAPI SHA: 5903bb39137fd76ac384b2044e425f9c56840e00, Date: 2023-10-23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants