Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

### Internal Changes
* Detect Databricks CLI version at init time via `databricks version --output json`, enabling version-gated flag support. Successful detections are cached per CLI path; subprocess failures fall back to the most conservative command and are retried on the next call.
* Pass `--force-refresh` to Databricks CLI `auth token` command (when the installed CLI is >= v0.296.0) so the SDK always receives a freshly minted token instead of a potentially stale one from the CLI's internal cache.

### API Changes
* Add `createExample()`, `deleteExample()`, `getExample()`, `getPermissionLevels()`, `getPermissions()`, `listExamples()`, `setPermissions()`, `updateExample()` and `updatePermissions()` methods for `workspaceClient.supervisorAgents()` service.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ public class DatabricksCliCredentialsProvider implements CredentialsProvider {
// --profile support added in CLI v0.207.1: https://github.com/databricks/cli/pull/855
static final DatabricksCliVersion CLI_VERSION_FOR_PROFILE = new DatabricksCliVersion(0, 207, 1);

// --force-refresh support added in CLI v0.296.0: https://github.com/databricks/cli/pull/4767
static final DatabricksCliVersion CLI_VERSION_FOR_FORCE_REFRESH =
new DatabricksCliVersion(0, 296, 0);
Comment thread
mihaimitrea-db marked this conversation as resolved.

// 5-second cap on `databricks version` so a hung CLI (slow first-run scan, antivirus, blocked
// stdin) does not wedge SDK init indefinitely.
private static final long VERSION_PROBE_TIMEOUT_SECONDS = 5;
Expand Down Expand Up @@ -161,13 +165,41 @@ List<String> resolveCliCommand(String cliPath, DatabricksConfig config) {
}

/**
* Builds the {@code auth token} command for the given CLI version.
* Builds the full {@code auth token} command, including capability-gated flags.
*
* <p>Falls back to {@code --host} when {@code --profile} is either not configured or not
* supported by the installed CLI.
* <p>Delegates the profile/host decision to {@link #buildCoreCliCommand} and appends {@code
* --force-refresh} when the installed CLI supports it.
*/
List<String> buildCliCommand(
String cliPath, DatabricksConfig config, DatabricksCliVersion version) {
List<String> cmd = buildCoreCliCommand(cliPath, config, version);
if (version.atLeast(CLI_VERSION_FOR_FORCE_REFRESH)) {
cmd.add("--force-refresh");
} else if (version.isDefaultDevBuild()) {
// Dev build β€” getCliVersion already emitted the dev-build INFO at the probe site.
} else if (version.equals(DatabricksCliVersion.UNKNOWN)) {
// We didn't actually prove the CLI lacks --force-refresh; we just failed to confirm it.
LOG.warn(
"Could not confirm --force-refresh support for Databricks CLI {} (requires >= {}). "
+ "The CLI's token cache may provide stale tokens.",
version,
CLI_VERSION_FOR_FORCE_REFRESH);
} else {
LOG.warn(
"Databricks CLI {} does not support --force-refresh (requires >= {}). "
+ "The CLI's token cache may provide stale tokens.",
version,
CLI_VERSION_FOR_FORCE_REFRESH);
Comment on lines +188 to +192
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Nit] Warning message has no actionable fix for the user.

"Databricks CLI v0.xxx does not support --force-refresh (requires >= v0.296.0). The CLI's token cache may provide stale tokens." - a user who sees this has no idea that the fix is to upgrade the CLI.

Suggestion: append an upgrade hint, e.g. "Upgrade via brew upgrade databricks or https://docs.databricks.com/dev-tools/cli/install.html to silence this warning." Match the phrasing used in the Go SDK's equivalent message for cross-SDK consistency.

Found by: Cursor.

}
return cmd;
}
Comment on lines +173 to +195
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Defensive copy before mutating the list from buildCoreCliCommand.

buildCliCommand mutates the list returned by buildCoreCliCommand via cmd.add("--force-refresh"). This works today only because buildCoreCliCommand and buildHostArgs happen to return ArrayList. A future refactor returning List.of(...) or Collections.unmodifiableList(...) will silently break this with UnsupportedOperationException at a call site that looks innocent.

Suggestion: either List<String> cmd = new ArrayList<>(buildCoreCliCommand(...)); at the top of this method, or document the mutability contract in javadoc on buildCoreCliCommand. The copy is more robust for a trivial cost.

Found by: Isaac.


/**
* Builds the base {@code auth token} command without capability-gated flags. Falls back to {@code
* --host} when {@code --profile} is either not configured or not supported by the installed CLI.
*/
List<String> buildCoreCliCommand(
String cliPath, DatabricksConfig config, DatabricksCliVersion version) {
if (config.getProfile() == null) {
return buildHostArgs(cliPath, config);
}
Expand All @@ -176,14 +208,7 @@ List<String> buildCliCommand(
// do not support it. Only use --profile in CLI versions known to support it in `auth token`.
if (!version.atLeast(CLI_VERSION_FOR_PROFILE)) {
if (version.isDefaultDevBuild()) {
// A default-marker dev build has no injected version, so every feature gate fails.
// Surface an informational hint so users know why their feature flags aren't taking
// effect.
LOG.info(
"Databricks CLI {} is a development build; feature detection will use conservative "
+ "fallbacks. Rebuild the CLI with an explicit version to enable capability-based "
+ "flag selection.",
version);
// Dev build β€” getCliVersion already emitted the dev-build INFO at the probe site.
} else if (version.equals(DatabricksCliVersion.UNKNOWN)) {
LOG.warn(
"Could not confirm --profile support for Databricks CLI {} (requires >= {}). "
Expand Down Expand Up @@ -254,11 +279,21 @@ DatabricksCliVersion getCliVersion(String cliPath, Environment env) {
LOG.warn(
"Failed to detect Databricks CLI version: {}. "
+ "Falling back to conservative flag set.",
e.getMessage(),
e);
e.getMessage());
LOG.debug("CLI version probe failure stack:", e);
return DatabricksCliVersion.UNKNOWN;
}
});
if (version.isDefaultDevBuild()) {
// A default-marker dev build has no injected version, so every feature gate falls back to
// the conservative path. Emit once at the probe site so callers see the explanation even
// on the host-only code path that never consults a per-flag gate.
LOG.info(
"Databricks CLI {} is a development build; feature detection will use conservative "
+ "fallbacks. Rebuild the CLI with an explicit version to enable capability-based "
+ "flag selection.",
version);
}
// Don't cache UNKNOWN: a transient probe failure or a parseable-but-malformed payload
// would otherwise pin every later token source to the conservative fallback for the rest
// of the process lifetime. Strip it after computeIfAbsent so the next call re-probes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockConstruction;
Expand All @@ -16,7 +16,6 @@
import com.databricks.sdk.core.utils.OSUtilities;
import com.databricks.sdk.core.utils.OSUtils;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
Expand Down Expand Up @@ -112,33 +111,56 @@ void testBuildCliCommand_ProfileWithNullHost_ThrowsClearError() {
private static Stream<Arguments> buildCliCommandCases() {
return Stream.of(
Arguments.of(
"host only β€” old CLI",
"host only β€” old CLI, no force-refresh",
new DatabricksConfig().setHost(HOST),
new DatabricksCliVersion(0, 200, 0),
Arrays.asList(CLI_PATH, "auth", "token", "--host", HOST)),
Arguments.of(
"account host β€” old CLI",
"host only β€” new CLI, with force-refresh",
new DatabricksConfig().setHost(HOST),
DatabricksCliCredentialsProvider.CLI_VERSION_FOR_FORCE_REFRESH,
Arrays.asList(CLI_PATH, "auth", "token", "--host", HOST, "--force-refresh")),
Arguments.of(
"account host β€” old CLI, no force-refresh",
new DatabricksConfig().setHost(ACCOUNT_HOST).setAccountId(ACCOUNT_ID),
new DatabricksCliVersion(0, 200, 0),
Arrays.asList(
CLI_PATH, "auth", "token", "--host", ACCOUNT_HOST, "--account-id", ACCOUNT_ID)),
Arguments.of(
"profile with new CLI β€” uses --profile",
"account host β€” new CLI, with force-refresh",
new DatabricksConfig().setHost(ACCOUNT_HOST).setAccountId(ACCOUNT_ID),
DatabricksCliCredentialsProvider.CLI_VERSION_FOR_FORCE_REFRESH,
Arrays.asList(
CLI_PATH,
"auth",
"token",
"--host",
ACCOUNT_HOST,
"--account-id",
ACCOUNT_ID,
"--force-refresh")),
Arguments.of(
"profile with profile-supporting CLI β€” uses --profile, no force-refresh",
new DatabricksConfig().setProfile(PROFILE).setHost(HOST),
DatabricksCliCredentialsProvider.CLI_VERSION_FOR_PROFILE,
Arrays.asList(CLI_PATH, "auth", "token", "--profile", PROFILE)),
Arguments.of(
"profile with old CLI β€” falls back to --host",
"profile with newest CLI β€” uses --profile and --force-refresh",
new DatabricksConfig().setProfile(PROFILE).setHost(HOST),
DatabricksCliCredentialsProvider.CLI_VERSION_FOR_FORCE_REFRESH,
Arrays.asList(CLI_PATH, "auth", "token", "--profile", PROFILE, "--force-refresh")),
Arguments.of(
"profile with old CLI β€” falls back to --host, no force-refresh",
new DatabricksConfig().setProfile(PROFILE).setHost(HOST),
new DatabricksCliVersion(0, 207, 0),
Arrays.asList(CLI_PATH, "auth", "token", "--host", HOST)),
Arguments.of(
"unknown version β€” falls back to --host",
"unknown version β€” falls back to --host, no force-refresh",
new DatabricksConfig().setProfile(PROFILE).setHost(HOST),
DatabricksCliVersion.UNKNOWN,
Arrays.asList(CLI_PATH, "auth", "token", "--host", HOST)),
Arguments.of(
"dev build β€” falls back to --host",
"dev build β€” falls back to --host, no force-refresh",
new DatabricksConfig().setProfile(PROFILE).setHost(HOST),
new DatabricksCliVersion(0, 0, 0),
Arrays.asList(CLI_PATH, "auth", "token", "--host", HOST)));
Expand Down Expand Up @@ -366,7 +388,6 @@ private static Process mockProcess(String stdout, int exitCode, boolean exited)
Process process = mock(Process.class);
when(process.getInputStream())
.thenReturn(new ByteArrayInputStream(stdout.getBytes(StandardCharsets.UTF_8)));
when(process.getOutputStream()).thenReturn(new ByteArrayOutputStream());
when(process.waitFor(anyLong(), any(TimeUnit.class))).thenReturn(exited);
when(process.exitValue()).thenReturn(exitCode);
// destroyForcibly() returns the Process so callers can chain .waitFor(...) on it.
Expand Down Expand Up @@ -398,7 +419,7 @@ void testProbeCliVersion_SuccessReturnsParsedVersion() throws Exception {
mockConstruction(
ProcessBuilder.class,
(pb, ctx) -> {
when(pb.redirectErrorStream(anyBoolean())).thenReturn(pb);
when(pb.redirectErrorStream(eq(true))).thenReturn(pb);
when(pb.start()).thenReturn(process);
})) {
mockedOSUtils.when(() -> OSUtils.get(any())).thenReturn(osUtils);
Expand Down Expand Up @@ -427,7 +448,7 @@ void testProbeCliVersion_TimeoutThrowsAndDestroys() throws Exception {
mockConstruction(
ProcessBuilder.class,
(pb, ctx) -> {
when(pb.redirectErrorStream(anyBoolean())).thenReturn(pb);
when(pb.redirectErrorStream(eq(true))).thenReturn(pb);
when(pb.start()).thenReturn(process);
})) {
mockedOSUtils.when(() -> OSUtils.get(any())).thenReturn(osUtils);
Expand All @@ -451,7 +472,7 @@ void testProbeCliVersion_NonZeroExitSurfacesOutput() throws Exception {
mockConstruction(
ProcessBuilder.class,
(pb, ctx) -> {
when(pb.redirectErrorStream(anyBoolean())).thenReturn(pb);
when(pb.redirectErrorStream(eq(true))).thenReturn(pb);
when(pb.start()).thenReturn(process);
})) {
mockedOSUtils.when(() -> OSUtils.get(any())).thenReturn(osUtils);
Expand Down
Loading