Call _resolve_host_metadata on Config init and make failures non-fatal#1318
Merged
hectorcast-db merged 2 commits intomainfrom Mar 11, 2026
Merged
Conversation
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
This was referenced Mar 11, 2026
hectorcast-db
force-pushed
the
hectorcast-db/stack/resolve-host-metadata-on-init
branch
from
df4fa68 to
238f0a5
Compare
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
force-pushed
the
hectorcast-db/stack/resolve-host-metadata-on-init
branch
from
238f0a5 to
63156f6
Compare
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 11, 2026
…e is_account_client docstrings (#1317) ## 🥞 Stacked PR Use this [link](https://github.com/databricks/databricks-sdk-py/pull/1317/files) to review incremental changes. - [**stack/use-test-environment-type-in-integration-tests**](#1317) [[Files changed](https://github.com/databricks/databricks-sdk-py/pull/1317/files)] - [stack/resolve-host-metadata-on-init](#1318) [[Files changed](https://github.com/databricks/databricks-sdk-py/pull/1318/files/0eff155d0ca7fc796d878cf9ee332dac5149be6a..63156f63eb80992d0c25a7b341d0b19b77dc9527)] - [stack/cloud-from-host-metadata](#1320) [[Files changed](https://github.com/databricks/databricks-sdk-py/pull/1320/files/63156f63eb80992d0c25a7b341d0b19b77dc9527..f9b0c3e2df72ddecb0fccb0d59af46f3a06846d2)] - [stack/resolve-token-audience-from-host-metadata](#1321) [[Files changed](https://github.com/databricks/databricks-sdk-py/pull/1321/files/f9b0c3e2df72ddecb0fccb0d59af46f3a06846d2..d97bd75a5692faddc36d51d46208513e442e7c87)] - [stack/google-auth-use-host-metadata](#1322) [[Files changed](https://github.com/databricks/databricks-sdk-py/pull/1322/files/d97bd75a5692faddc36d51d46208513e442e7c87..87b4c1a35e3fe1abfb125069ba1590eeaecf34e9)] --------- ## Summary Replace `is_account_client` heuristics and `DATABRICKS_ACCOUNT_ID` presence checks in integration test fixtures with explicit `TEST_ENVIRONMENT_TYPE`-based filtering. Also deprecate `is_account_client` in docstrings across `Config` and `ApiClient`. ## Why Integration test fixtures in `tests/integration/conftest.py` used two different heuristics to decide whether a test should run: 1. `account_client.config.is_account_client` — inspects the host URL to infer whether the SDK is talking to an account-level API. This is unreliable now that unified hosts can serve both account and workspace APIs, and the property already raises `ValueError` when `experimental_is_unified_host` is set. 2. `"DATABRICKS_ACCOUNT_ID" in os.environ` — a proxy check used to skip workspace fixtures on account environments. This is fragile and doesn't express intent clearly. The test orchestrator now sets `TEST_ENVIRONMENT_TYPE` explicitly to one of `ACCOUNT`, `UC_ACCOUNT`, `WORKSPACE`, or `UC_WORKSPACE`. Using this value directly makes fixture skip-guards authoritative and easy to reason about. ## What changed ### Interface changes None. No public SDK API changes. ### Behavioral changes None for SDK users. Integration test fixture skip conditions are now driven by `TEST_ENVIRONMENT_TYPE` instead of `is_account_client` and `DATABRICKS_ACCOUNT_ID`. ### Internal changes - **`tests/integration/conftest.py`**: All four client fixtures (`a`, `ucacct`, `w`, `ucws`) now check `TEST_ENVIRONMENT_TYPE` against an explicit allowlist instead of using `is_account_client` or `DATABRICKS_ACCOUNT_ID`. - **`databricks/sdk/config.py`**: Updated `is_account_client` docstring to note deprecation — clients can now support both workspace and account APIs from a single host. - **`databricks/sdk/core.py`**: Added matching deprecation note to `ApiClient.is_account_client`. ## How is this tested? The changed code is test infrastructure itself. Correctness will be validated when integration tests run against environments that set `TEST_ENVIRONMENT_TYPE`. NO_CHANGELOG=true
hectorcast-db
force-pushed
the
hectorcast-db/stack/resolve-host-metadata-on-init
branch
from
63156f6 to
cd35488
Compare
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
approved these changes
Mar 11, 2026
Contributor
tejaskochar-db
left a comment
tejaskochar-db
left a comment
There was a problem hiding this comment.
lgtm modulo question about removing existing tests already, while we're gating the unified behaviour with a flag.
| "workspace_id": _DUMMY_WORKSPACE_ID, | ||
| }, | ||
| {}, | ||
| {"experimental_is_unified_host": True}, |
Contributor
There was a problem hiding this comment.
we're replacing a test for the general case with a test for the experimental flag case. Shouldn't we have both until the flag is removed?
Contributor
Author
There was a problem hiding this comment.
Actually, this test was always for the unified path.
The always called the resolve_metadata_path, but now this is skipped if the flag is false.
| _DUMMY_ACC_HOST, | ||
| {"oidc_endpoint": f"{_DUMMY_ACC_HOST}/oidc/accounts/{{account_id}}"}, | ||
| {"account_id": _DUMMY_ACCOUNT_ID}, | ||
| {"account_id": _DUMMY_ACCOUNT_ID, "experimental_is_unified_host": True}, |
| { | ||
| "account_id": _DUMMY_ACCOUNT_ID, | ||
| "workspace_id": _DUMMY_WORKSPACE_ID, | ||
| "experimental_is_unified_host": True, |
hectorcast-db
deleted the
hectorcast-db/stack/resolve-host-metadata-on-init
branch
github-merge-queue bot
pushed a commit
to databricks/databricks-sdk-go
that referenced
this pull request
Mar 16, 2026
## 🥞 Stacked PR Use this [link](https://github.com/databricks/databricks-sdk-go/pull/1542/files) to review incremental changes. - [**stack/port/resolve-host-metadata-on-init**](#1542) [[Files changed](https://github.com/databricks/databricks-sdk-go/pull/1542/files)] - [stack/port/resolve-token-audience-from-metadata](#1543) [[Files changed](https://github.com/databricks/databricks-sdk-go/pull/1543/files/20b6cd4abc1a3284d586c88f802c4b7df2678062..9893d9cbbfe8baab7f7aeacb8ce7faf49026c86a)] - [stack/port/gcp-sa-token-non-blocking](#1544) [[Files changed](https://github.com/databricks/databricks-sdk-go/pull/1544/files/9893d9cbbfe8baab7f7aeacb8ce7faf49026c86a..07e28b7aef05ada2f357f87faa749c6990be8173)] - [stack/port/test-environment-type](#1545) [[Files changed](https://github.com/databricks/databricks-sdk-go/pull/1545/files/07e28b7aef05ada2f357f87faa749c6990be8173..0da1b0d546ab8842dffbd50aa55fb136bbeffddf)] - [stack/port/host-metadata-integration-test](#1546) [[Files changed](https://github.com/databricks/databricks-sdk-go/pull/1546/files/0da1b0d546ab8842dffbd50aa55fb136bbeffddf..e9854aad19dc522ffe8def175bef3a3eabface2b)] - [stack/port/remove-unified-flag](#1547) [[Files changed](https://github.com/databricks/databricks-sdk-go/pull/1547/files/e9854aad19dc522ffe8def175bef3a3eabface2b..fae626deb92c4671a0c8aa0f1e3e6bad1f8c5cc6)] - [stack/port/gcp-sa-from-metadata](#1548) [[Files changed](https://github.com/databricks/databricks-sdk-go/pull/1548/files/fae626deb92c4671a0c8aa0f1e3e6bad1f8c5cc6..ecb1dbeed4ed1990a74895c6ced958c05f16ffef)] --------- ## Summary - Port of Python SDK PR databricks/databricks-sdk-py#1318 and discovery URL fix from PR databricks/databricks-sdk-py#1332 - Extract `applyHostMetadata()` from `resolveHostMetadata()` for reuse during config init - Call host metadata resolution during `EnsureResolved()` for unified hosts (gated behind `Experimental_IsUnifiedHost`), with non-fatal error handling (warns on failure) - OIDC endpoint from metadata is now treated as the OIDC root, with `/.well-known/oauth-authorization-server` appended to form the full discovery URL ## Test plan - `TestEnsureResolved_ResolvesHostMetadata_WhenUnifiedHost` — verifies fields populated from metadata - `TestEnsureResolved_HostMetadataFailure_NonFatal` — 500 response, config still resolves - `TestEnsureResolved_HostMetadata_NoOidcEndpoint_NonFatal` — missing oidc_endpoint, no error - `TestEnsureResolved_HostMetadata_MissingAccountIdWithPlaceholder_Warns` — template needs account_id but missing - Existing `resolveHostMetadata` tests updated for new discovery URL format NO_CHANGELOG=true This pull request was AI-assisted by Isaac.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
🥞 Stacked PR
Use this link to review incremental changes.
Summary
Call
_resolve_host_metadata()automatically duringConfig.__init__so thataccount_id,workspace_id,discovery_url, andcloudare populated from the/.well-known/databricks-configendpoint before auth is initialized. Metadata fetch failures are non-fatal — a warning is logged and initialization continues with whatever the user has explicitly configured.Why
_resolve_host_metadata()already existed but had to be called explicitly by callers who wanted auto-discovery. This meant thatWorkspaceClientandAccountClientdid not benefit from it out of the box. Beyond the missing call, the old implementation was also strictly fatal: any HTTP error or missing field (account_id,oidc_endpoint) raised aValueError, making it unsafe to run automatically since not all hosts expose the endpoint.This PR wires up the call during
Config.__init__(between_fix_host_if_neededand_validate) and hardens the implementation so it degrades gracefully: HTTP failures produce a warning, and missing optional fields are simply left unset. The only remaining hard failure is when a{account_id}template inoidc_endpointcannot be substituted because noaccount_idis available — this is a configuration error that should still surface.Auto-discovery is currently gated to hosts with
experimental_is_unified_host=True(a# TODO: Enable this everywheremarks the guard) to avoid unintended network calls on standard workspace and account deployments until the endpoint is broadly available.What changed
Interface changes
None.
_resolve_host_metadata()remains private.Behavioral changes
For hosts with
experimental_is_unified_host=True:Config.__init__now makes an HTTP request to{host}/.well-known/databricks-configand silently back-fillsaccount_id,workspace_id,discovery_url, andcloudfrom the response. Previously this did not happen.All other hosts: no change.
Internal changes
databricks/sdk/config.py: Added_resolve_host_metadata()call toConfig.__init__. AddedHostType.UNIFIEDguard. Made HTTP failures non-fatal (warning + return). Removed mandatory-field assertions (account_id is required,discovery_url is required) — missing fields are now silently skipped.tests/conftest.py: Added autousestub_host_metadatafixture that mocksget_host_metadatafor all unit tests, preventing unexpected network calls.tests/test_config.py: Replaced the oldrequests_mock-based tests (which called_resolve_host_metadata()directly) withmocker.patch-based tests that exercise the fullConfig.__init__path. Addedtest_resolve_host_metadata_skipped_for_non_unifiedto assert the guard works.How is this tested?
Unit tests in
tests/test_config.pycover: successful resolution of all fields, no-overwrite of pre-configured values, HTTP failure tolerance, missingoidc_endpoint, and the guard that skips non-unified hosts.NO_CHANGELOG=true