Skip to content

Commit

Permalink
Fix handling of bare DATABRICKS_HOST environment variables.
Browse files Browse the repository at this point in the history
Some environments (e.g. Workbench) set `DATABRICKS_HOST` to the bare
hostname and omit the HTTPS prefix; this breaks when we later expect it
to be an actual URL.

So this commit updates `databricks_workspace()` to handle both cases.
Similar code exists in all of the other Databricks packages and SDKs
that I am aware of, so this is nothing new; I just missed it.

Unit tests are included.

Signed-off-by: Aaron Jacobs <[email protected]>
  • Loading branch information
atheriel committed Jan 15, 2025
1 parent 587d627 commit d1bdd8e
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 1 deletion.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
* `chat_azure()` now has a `credentials` argument to make authentication against
Azure more flexible (#248, @atheriel).

* `chat_databricks()` now handles the `DATABRICKS_HOST` environment variable
correctly whether it includes an HTTPS prefix or not (#252, @atheriel).

# ellmer 0.1.0

* New `chat_vllm()` to chat with models served by vLLM (#140).
Expand Down
6 changes: 5 additions & 1 deletion R/provider-databricks.R
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,11 @@ method(as_json, list(ProviderDatabricks, ContentText)) <- function(provider, x)
}

databricks_workspace <- function() {
key_get("DATABRICKS_HOST")
host <- key_get("DATABRICKS_HOST")
if (!is.null(host) && !grepl("^https?://", host)) {
host <- paste0("https://", host)
}
host
}

# Try various ways to get Databricks credentials. This implements a subset of
Expand Down
17 changes: 17 additions & 0 deletions tests/testthat/test-provider-databricks.R
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,20 @@ test_that("M2M authentication requests look correct", {
})
expect_equal(databricks_token(), "token")
})

test_that("workspace detection handles URLs with and without an https prefix", {
withr::with_envvar(
c(DATABRICKS_HOST = "example.cloud.databricks.com"),
expect_equal(
databricks_workspace(),
"https://example.cloud.databricks.com"
)
)
withr::with_envvar(
c(DATABRICKS_HOST = "https://example.cloud.databricks.com"),
expect_equal(
databricks_workspace(),
"https://example.cloud.databricks.com"
)
)
})

0 comments on commit d1bdd8e

Please sign in to comment.