From d1bdd8e8555bafa9b417c2e995bb0bd3afa78905 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 14 Jan 2025 19:56:27 -0500 Subject: [PATCH] Fix handling of bare DATABRICKS_HOST environment variables. 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 --- NEWS.md | 3 +++ R/provider-databricks.R | 6 +++++- tests/testthat/test-provider-databricks.R | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index eebabc0..d0cd95b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -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). diff --git a/R/provider-databricks.R b/R/provider-databricks.R index ecc944d..60c8252 100644 --- a/R/provider-databricks.R +++ b/R/provider-databricks.R @@ -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 diff --git a/tests/testthat/test-provider-databricks.R b/tests/testthat/test-provider-databricks.R index 7584488..e932e02 100644 --- a/tests/testthat/test-provider-databricks.R +++ b/tests/testthat/test-provider-databricks.R @@ -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" + ) + ) +})