Added `w.config.account_host` to get the relevant account host from a workspace client by nfx · Pull Request #390 · databricks/databricks-sdk-py

nfx · 2023-10-06T18:11:37Z

Changes

This simplifies downstream integration testing as well as configuration

Tests

make test run locally
make fmt applied
relevant integration tests applied

codecov-commenter · 2023-10-06T18:13:30Z

Codecov Report

All modified lines are covered by tests ✅

Files	Coverage Δ
databricks/sdk/azure.py	`100.00% <100.00%> (ø)`
databricks/sdk/core.py	`80.69% <100.00%> (+0.32%)`	⬆️

📢 Thoughts on this report? Let us know!.

mgyucht · 2023-10-12T13:07:19Z

databricks/sdk/azure.py

-                            active_directory_endpoint="https://login.microsoftonline.de/"),
+    GERMAN=AzureEnvironment(
+        name="AzureGermanCloud",
+        databricks_hostname_suffix='.databricks.azure.de', # TODO: i'm not sure about this one


I think this is OK because we don't run Databricks in this cloud (and it is being deprecated: https://learn.microsoft.com/en-us/previous-versions/azure/germany/germany-welcome)

mgyucht · 2023-10-12T13:12:48Z

databricks/sdk/core.py

        return self.host.startswith("https://accounts.") or self.host.startswith("https://accounts-dod.")

+    @property
+    def account_host(self) -> str:


This won't work for dev/staging, right? We occasionally get internal issues where the SDK doesn't work well with dev/staging.

Also, can we label this somehow as experimental? In the future, if we introduce a workspace-level metadata service, the accounts endpoint can be exposed there, and we can remove this property.

can you suggest on the pattern here to make it work for dev/staging?

btw, once we introduce workspace-level metadata service, we should just print warnings from within this call.

One idea: have a list to lookup cloud/env/account host name based on domain suffix, from most-specific to least-specific:

# Most specific to least specific [ ('.dev.databricks.com', 'AWS', 'DEV', 'PUBLIC', 'accounts.dev.databricks.com'), ('.staging.cloud.databricks.com', 'AWS', 'STAGING', 'PUBLIC', 'accounts.staging.cloud.databricks.com'), ('.cloud.databricks.com', 'AWS', 'PROD', 'PUBLIC', 'accounts.cloud.databricks.com'), ('.cloud.databricks.us', 'AWS', 'PROD', 'GOV', 'accounts.cloud.databricks.us'), ('.dev.azuredatabricks.net', 'AZURE', 'DEV', 'PUBLIC', 'accounts.dev.azuredatabricks.net') ('.staging.azuredatabricks.net', 'AZURE', 'STAGING', 'PUBLIC', 'accounts.staging.azuredatabricks.net'), ('.azuredatabricks.net', 'AZURE', 'PROD', 'PUBLIC', 'accounts.azuredatabricks.net'), ('.databricks.azure.us', 'AZURE', 'PROD', 'GOV', 'accounts.databricks.azure.us'), ('.dev.gcp.databricks.com', 'GCP', 'DEV', 'PUBLIC', 'accounts.dev.gcp.databricks.com'), ('.staging.gcp.databricks.com', 'GCP', 'STAGING', 'PUBLIC', 'accounts.staging.gcp.databricks.com'), ('.gcp.databricks.com', 'GCP', 'PROD', 'PUBLIC', 'accounts.gcp.databricks.com'), ]

The simpler version is to just replace the most specific zone with accounts, which works everywhere today.

@mgyucht do we want to expose dev/staging?... if we do - then why don't we expose the azure_login_app_id along with it?

That's definitely fine by me!

mgyucht · 2023-10-12T13:14:12Z

One nit to make this work for dev/staging & label as experimental, but otherwise this seems reasonable to me if it unblocks you/customers.

hamza-db · 2023-11-08T20:49:53Z

all the info here is fine to be public ( and already public)

mgyucht

This is great, thank you for contributing! Couple nits/suggestions, otherwise this looks good to me.

mgyucht · 2023-11-09T08:54:18Z

databricks/sdk/core.py

        return self.host.startswith("https://accounts.") or self.host.startswith("https://accounts-dod.")

+    @property
+    def account_host(self) -> str:


That's definitely fine by me!

databricks/sdk/core.py

mgyucht · 2023-11-09T08:56:07Z

databricks/sdk/core.py

+    cloud: Cloud
+
+    # domain suffixes are not very secret: https://crt.sh/?q=databricks
+    tld: str


nit: let's call this dns_zone (I think TLD would traditionally refer to com. or net.)

dns_zone it is then.

mgyucht · 2023-11-09T09:01:38Z

databricks/sdk/core.py

+    def deployment(self, deployment_name: str) -> str:
+        return f'https://{deployment_name}{self.tld}'


This is maybe just a bit abstract. It won't work in Azure the way a user might expect, where the DNS name can't be determined from the workspace ID alone. Given we're only using this for account host, I suggest we just inline it there.

it's already part of our response:

for now, we work it around in https://github.com/databrickslabs/ucx/blob/main/src/databricks/labs/ucx/account/workspaces.py#L101-L126, but @iaroslav-db fixed the mapping so that we don't have to work it around soon ;)

I need this function to create a workspace client from an account client. See:

https://github.com/databrickslabs/watchdog/blob/main/scan/scan.go#L144-L154

https://github.com/databrickslabs/ucx/blob/main/src/databricks/labs/ucx/account/workspaces.py#L122-L126

https://github.com/databricks/terraform-provider-databricks/blob/baa61c451055271e14e7a9bc3db43537dcfda8ea/mws/resource_mws_workspaces.go#L372-L377

I'll do a follow-up PR to account client once azure changes are rolled out to prod

Oh perfect, I actually didn't realize those workspace APIs worked in azure yet.

… workspace client This simplifies downstream integration testing as well as configuration introduce `DatabricksEnvironment` ..

nfx · 2023-11-09T11:10:35Z

this needs a bit more adjustments to work

pietern · 2023-11-09T12:22:34Z

Are similar PRs for the other SDKs tracked some place?

nfx · 2023-11-10T16:36:17Z

@pietern i'll have one for GoLang the following week - once i figure out the way to have it friendly for unit tests, need the same thing for two projects

…ere IMDS doesn't give host environment information (#700) ## Changes This PR allows determining Azure Environment from a Databricks account or workspace hostname, removing the need for a separate configuration/environment variable and complexities related to Azure MSI from within ACR. Similar functionality in Python SDK: - databricks/databricks-sdk-py#390 Stacked on top of: - #699 ## Tests  - [x] `make test` passing - [x] `make fmt` applied - [ ] relevant integration tests applied

renaudhartert-db · 2026-03-27T14:15:05Z

Thanks for this contribution. We're closing this PR as part of regular housekeeping since it has been inactive for over a year. This is not a reflection on the quality of the work -- if the changes are still relevant, feel free to re-open the PR or open a new one and we'll be happy to review it.

nfx added the chore anything that makes the code better label Oct 6, 2023

nfx requested a review from a team October 6, 2023 18:11

mgyucht reviewed Oct 12, 2023

View reviewed changes

mgyucht approved these changes Nov 9, 2023

View reviewed changes

Added w.config.account_host to get the relevant account host from a…

cb830fc

… workspace client This simplifies downstream integration testing as well as configuration introduce `DatabricksEnvironment` ..

nfx force-pushed the feat/account-host branch from bea774f to cb830fc Compare November 9, 2023 09:21

nfx added the enhancement New feature or request label Nov 9, 2023

nfx added 2 commits November 9, 2023 10:40

..

a152dd4

... some changes

f134fe1

nfx added the do-not-merge label Nov 9, 2023

nfx mentioned this pull request Nov 20, 2023

Introduce DatabricksEnvironment and fix Azure MSI auth from ACR, where IMDS doesn't give host environment information databricks/databricks-sdk-go#700

Merged

3 tasks

nfx mentioned this pull request Nov 28, 2023

Added task filtering by cloud databrickslabs/ucx#630

Closed

renaudhartert-db closed this Mar 27, 2026

		def deployment(self, deployment_name: str) -> str:
		return f'https://{deployment_name}{self.tld}'

Conversation

nfx commented Oct 6, 2023

Changes

Tests

Uh oh!

codecov-commenter commented Oct 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgyucht commented Oct 12, 2023

Uh oh!

hamza-db commented Nov 8, 2023

Uh oh!

mgyucht left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfx Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfx commented Nov 9, 2023

Uh oh!

pietern commented Nov 9, 2023

Uh oh!

nfx commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

renaudhartert-db commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented Oct 6, 2023 •

edited

Loading

nfx Nov 9, 2023 •

edited

Loading

nfx commented Nov 10, 2023 •

edited

Loading