diff --git a/packages/shared/src/cli/commands/doctor/README.md b/packages/shared/src/cli/commands/doctor/README.md index 85a519906..c360a3de6 100644 --- a/packages/shared/src/cli/commands/doctor/README.md +++ b/packages/shared/src/cli/commands/doctor/README.md @@ -14,7 +14,7 @@ so the reported problem is the *root* cause, not a symptom. | ------------ | ----------------------------------------------------- | ---------------------------------------------------------- | | `auth` | Can we authenticate to the workspace at all? | validate `DATABRICKS_HOST` is a real URL, then `currentUser.me()` — once, app-wide; a failure skips the live layer | | `config` | Are the resource's field env vars **present**? | offline presence check of `process.env` (presence only — see note) | -| `existence` | Does the resource exist and is it reachable? | cheapest per-type live probe (`warehouses.get`, `servingEndpoints.get`, …); Lakebase runs a real `SELECT 1` | +| `existence` | Does the resource exist and is it reachable? | cheapest per-type live probe (`warehouses.getWarehouse`, `GET /api/2.0/serving-endpoints/{name}`, …); Lakebase runs a real `SELECT 1` | `config` checks env-var presence only; whether a value points at a real resource is the `existence` layer's job. (`DATABRICKS_HOST` is the exception — `auth` @@ -217,15 +217,16 @@ skipped outright, not merged.) Because of that, a failed or conflicted auth row reports **both** `host:` and `profile:`. The host shown is the one the SDK actually resolved, read from -`client.config.host` — which the SDK populates lazily on the first API call, so -it's read *after* `me()`, not at construction. When the client never got built, -the host is recovered from the `host=…` fragment the SDK appends to its -`ConfigError`; failing that, it falls back to `DATABRICKS_HOST`. Every path is -passed through `sanitizeHost`, so embedded `user:pass@` credentials can't leak. +`client.getHost()`. That resolves from env + profile only (no network), so it's +available even when the credentials are rejected. When resolution itself fails +(missing profile, no host anywhere), it falls back to `DATABRICKS_HOST`. Every +path is passed through `sanitizeHost`, so embedded `user:pass@` credentials +can't leak. `HOST_PROFILE_CONFLICT` compares `DATABRICKS_HOST` against the profile's own -declared host, read offline from `~/.databrickscfg` via the SDK's exported -`loadConfigFile` (the resolved config is useless here — env has already won). +declared host, read offline from `~/.databrickscfg` via sdk-core's profile +`resolve` with the env overlay disabled (the resolved config is useless here — +env has already won). Comparison ignores scheme, case, and trailing slash. It's a **warning**, not an error: the credentials do work, so it must not gate CI, but a green tick would hide a real misconfiguration. When auth *also* fails, the conflict replaces the diff --git a/packages/shared/src/cli/commands/doctor/checks-existence.test.ts b/packages/shared/src/cli/commands/doctor/checks-existence.test.ts index f4adb3fe9..9ed0ca2bb 100644 --- a/packages/shared/src/cli/commands/doctor/checks-existence.test.ts +++ b/packages/shared/src/cli/commands/doctor/checks-existence.test.ts @@ -35,7 +35,7 @@ function apiError(statusCode: number, message = "boom"): Error { describe("runExistenceProbe — sql_warehouse", () => { it("ok when the warehouse exists and is RUNNING", async () => { const client = { - warehouses: { get: async () => ({ state: "RUNNING" }) }, + warehouses: { getWarehouse: async () => ({ state: "RUNNING" }) }, }; const r = await runExistenceProbe(client, target()); expect(r.status).toBe("ok"); @@ -43,7 +43,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("warns when the warehouse exists but is STOPPED", async () => { const client = { - warehouses: { get: async () => ({ state: "STOPPED" }) }, + warehouses: { getWarehouse: async () => ({ state: "STOPPED" }) }, }; const r = await runExistenceProbe(client, target()); expect(r.status).toBe("warn"); @@ -53,7 +53,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("errors NOT_FOUND on a 404", async () => { const client = { warehouses: { - get: async () => { + getWarehouse: async () => { throw apiError(404); }, }, @@ -66,7 +66,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("errors INVALID_VALUE on a 400, quoting the value (no type/blob leak)", async () => { const client = { warehouses: { - get: async () => { + getWarehouse: async () => { throw Object.assign( new Error( 'Response from server (Bad Request) {"error_code":"INVALID_PARAMETER_VALUE","message":"bogus is not a valid endpoint id."}', @@ -90,7 +90,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("errors ACCESS_DENIED on a 403", async () => { const client = { warehouses: { - get: async () => { + getWarehouse: async () => { throw apiError(403); }, }, @@ -103,7 +103,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("hedges on a 403, which several APIs also return for a missing resource", async () => { const client = { warehouses: { - get: async () => { + getWarehouse: async () => { throw apiError(403); }, }, @@ -114,8 +114,31 @@ describe("runExistenceProbe — sql_warehouse", () => { expect(r.detail).toContain("wh-1"); }); + // The modular SDK's ApiError carries `httpStatusCode` / `code`, not the + // legacy `statusCode` / `errorCode`; both must classify the same. + it("classifies the modular ApiError shape (httpStatusCode / code)", async () => { + const modular = (httpStatusCode: number, code: string) => ({ + getWarehouse: async () => { + throw Object.assign(new Error("boom"), { httpStatusCode, code }); + }, + }); + const cases: Array<[number, string, string]> = [ + [404, "RESOURCE_DOES_NOT_EXIST", "NOT_FOUND"], + [400, "INVALID_PARAMETER_VALUE", "INVALID_VALUE"], + [403, "PERMISSION_DENIED", "ACCESS_DENIED"], + [-1, "RESOURCE_DOES_NOT_EXIST", "NOT_FOUND"], + ]; + for (const [status, code, expected] of cases) { + const r = await runExistenceProbe( + { warehouses: modular(status, code) }, + target(), + ); + expect(r.code, `${status}/${code}`).toBe(expected); + } + }); + it("skips when the id field is missing", async () => { - const client = { warehouses: { get: async () => ({}) } }; + const client = { warehouses: { getWarehouse: async () => ({}) } }; const r = await runExistenceProbe(client, target({ fieldValues: {} })); expect(r.status).toBe("skipped"); expect(r.code).toBe("MISSING_FIELD"); @@ -124,7 +147,7 @@ describe("runExistenceProbe — sql_warehouse", () => { describe("runExistenceProbe — job", () => { it("errors on a non-integer job id", async () => { - const client = { jobs: { get: async () => ({}) } }; + const client = { jobs: { getJob: async () => ({}) } }; const r = await runExistenceProbe( client, target({ type: "job", fieldValues: { id: "not-a-number" } }), @@ -134,7 +157,7 @@ describe("runExistenceProbe — job", () => { }); it("ok for a valid integer job id", async () => { - const client = { jobs: { get: async () => ({}) } }; + const client = { jobs: { getJob: async () => ({}) } }; const r = await runExistenceProbe( client, target({ type: "job", fieldValues: { id: "42" } }), @@ -149,8 +172,8 @@ describe("runExistenceProbe — job", () => { let seen: unknown; const client = { jobs: { - get: async (req: { job_id: unknown }) => { - seen = req.job_id; + getJob: async (req: { jobId: unknown }) => { + seen = req.jobId; return {}; }, }, @@ -164,7 +187,7 @@ describe("runExistenceProbe — job", () => { }); it("rejects numeric forms the API can't take (1e3, 0x10)", async () => { - const client = { jobs: { get: async () => ({}) } }; + const client = { jobs: { getJob: async () => ({}) } }; for (const id of ["1e3", "0x10", "4.5", "-1"]) { const r = await runExistenceProbe( client, @@ -260,7 +283,7 @@ describe("runExistenceProbe — postgres (Lakebase)", () => { describe("runExistenceProbe — per-type coverage with real manifest keys", () => { it("serving_endpoint: ok via `name`", async () => { - const client = { servingEndpoints: { get: async () => ({}) } }; + const client = { request: async () => ({}) }; const r = await runExistenceProbe( client, target({ @@ -273,10 +296,8 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = it("serving_endpoint: hints id-vs-name when configured by id and the probe fails", async () => { const client = { - servingEndpoints: { - get: async () => { - throw Object.assign(new Error("not found"), { statusCode: 404 }); - }, + request: async () => { + throw Object.assign(new Error("not found"), { statusCode: 404 }); }, }; const r = await runExistenceProbe( @@ -289,10 +310,8 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = it("serving_endpoint: no id-vs-name hint when configured by name", async () => { const client = { - servingEndpoints: { - get: async () => { - throw Object.assign(new Error("not found"), { statusCode: 404 }); - }, + request: async () => { + throw Object.assign(new Error("not found"), { statusCode: 404 }); }, }; const r = await runExistenceProbe( @@ -304,7 +323,7 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }); it("genie_space: ok via `id`", async () => { - const client = { genie: { getSpace: async () => ({}) } }; + const client = { genie: { genieGetSpace: async () => ({}) } }; const r = await runExistenceProbe( client, target({ type: "genie_space", fieldValues: { id: "01ef" } }), @@ -313,7 +332,7 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }); it("volume: ok via `path` (real manifest key)", async () => { - const client = { volumes: { read: async () => ({}) } }; + const client = { request: async () => ({}) }; const r = await runExistenceProbe( client, target({ @@ -325,7 +344,7 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }); it("uc_function: ok via `name`", async () => { - const client = { functions: { get: async () => ({}) } }; + const client = { request: async () => ({}) }; const r = await runExistenceProbe( client, target({ type: "uc_function", fieldValues: { name: "cat.sch.fn" } }), @@ -334,8 +353,8 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }); it("vector_search_index: probes via camelCase `indexName`", async () => { - const getIndex = vi.fn(async () => ({})); - const client = { vectorSearchIndexes: { getIndex } }; + const request = vi.fn(async () => ({})); + const client = { request }; const r = await runExistenceProbe( client, target({ @@ -344,11 +363,14 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }), ); expect(r.status).toBe("ok"); - expect(getIndex).toHaveBeenCalledWith({ index_name: "main.default.idx" }); + expect(request).toHaveBeenCalledWith({ + method: "GET", + path: "/api/2.0/vector-search/indexes/main.default.idx", + }); }); it("vector_search_index: skips MISSING_FIELD when index name absent", async () => { - const client = { vectorSearchIndexes: { getIndex: async () => ({}) } }; + const client = { request: async () => ({}) }; const r = await runExistenceProbe( client, target({ type: "vector_search_index", fieldValues: {} }), diff --git a/packages/shared/src/cli/commands/doctor/checks-existence.ts b/packages/shared/src/cli/commands/doctor/checks-existence.ts index 10a63e2f1..cf07f13bd 100644 --- a/packages/shared/src/cli/commands/doctor/checks-existence.ts +++ b/packages/shared/src/cli/commands/doctor/checks-existence.ts @@ -2,7 +2,7 @@ * Layer: existence — per-resource-type probes that prove a declared resource * exists and is reachable via the cheapest read the SDK offers. * - * The client is typed structurally (not via the SDK) so `shared` stays SDK-free. + * The client is typed structurally (the facade's modular clients + `request()`). */ import { @@ -18,26 +18,29 @@ const EXISTENCE_OK: LayerResult = { layer: "existence", status: "ok" }; interface DoctorWorkspaceClient { warehouses: { - get: (r: { id: string }) => Promise<{ state?: string }>; - }; - servingEndpoints: { - get: (r: { name: string }) => Promise; + getWarehouse: (r: { id: string }) => Promise<{ state?: string }>; }; genie: { - getSpace: (r: { space_id: string }) => Promise; + genieGetSpace: (r: { spaceId: string }) => Promise; }; jobs: { - get: (r: { job_id: number }) => Promise; - }; - volumes: { - read: (r: { name: string }) => Promise; - }; - vectorSearchIndexes: { - getIndex: (r: { index_name: string }) => Promise; - }; - functions: { - get: (r: { name: string }) => Promise; + getJob: (r: { jobId: bigint }) => Promise; }; + /** Raw REST GET for services the facade has no modular client for yet + * (serving, volumes, vector search, UC functions). Throws on non-2xx. */ + request: (r: { method: string; path: string }) => Promise; +} + +/** A REST `GET` on a resource path, with the name URL-encoded. */ +function getResource( + client: DoctorWorkspaceClient, + basePath: string, + name: string, +): Promise { + return client.request({ + method: "GET", + path: `${basePath}/${encodeURIComponent(name)}`, + }); } type ExistenceProbe = ( @@ -45,21 +48,21 @@ type ExistenceProbe = ( target: ResourceTarget, ) => Promise; -// Read statusCode/errorCode off the SDK's ApiError structurally. +// Read the HTTP status / Databricks error code off an ApiError structurally. +// Two shapes reach here: the wrapper's `ApiError` (from `request()`: +// `statusCode` / `errorCode`) and the modular SDK's (`httpStatusCode` / `code`). function statusCodeOf(err: unknown): number | undefined { - if (err && typeof err === "object" && "statusCode" in err) { - const code = (err as { statusCode?: unknown }).statusCode; - if (typeof code === "number") return code; - } - return undefined; + if (!err || typeof err !== "object") return undefined; + const e = err as { statusCode?: unknown; httpStatusCode?: unknown }; + const code = e.statusCode ?? e.httpStatusCode; + return typeof code === "number" && code >= 0 ? code : undefined; } function errorCodeOf(err: unknown): string | undefined { - if (err && typeof err === "object" && "errorCode" in err) { - const code = (err as { errorCode?: unknown }).errorCode; - if (typeof code === "string" && code.length > 0) return code; - } - return undefined; + if (!err || typeof err !== "object") return undefined; + const e = err as { errorCode?: unknown; code?: unknown }; + const code = e.errorCode ?? e.code; + return typeof code === "string" && code.length > 0 ? code : undefined; } // The SDK message often embeds a JSON blob; pull the inner `message` out so @@ -153,7 +156,7 @@ const probeWarehouse: ExistenceProbe = async (client, target) => { const id = field(target, "id"); if (!id) return missingField("id"); try { - const wh = await client.warehouses.get({ id }); + const wh = await client.warehouses.getWarehouse({ id }); const state = wh.state; if (state && state !== "RUNNING") { return { @@ -177,7 +180,7 @@ const probeServing: ExistenceProbe = async (client, target) => { const value = name ?? idOnly; if (!value) return missingField("name"); try { - await client.servingEndpoints.get({ name: value }); + await getResource(client, "/api/2.0/serving-endpoints", value); return EXISTENCE_OK; } catch (err) { const result = classifyError(err, target); @@ -193,7 +196,7 @@ const probeGenie: ExistenceProbe = async (client, target) => { const spaceId = field(target, "id"); if (!spaceId) return missingField("id"); try { - await client.genie.getSpace({ space_id: spaceId }); + await client.genie.genieGetSpace({ spaceId }); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); @@ -217,10 +220,8 @@ const probeJob: ExistenceProbe = async (client, target) => { }; } try { - // Pass the digits through unconverted to preserve int64 precision; the SDK - // serialises them straight into the request, so a string works even though - // the type says number. - await client.jobs.get({ job_id: id as unknown as number }); + // BigInt keeps int64 precision (the digits were validated above). + await client.jobs.getJob({ jobId: BigInt(id) }); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); @@ -242,7 +243,7 @@ const probeVolume: ExistenceProbe = async (client, target) => { }; } try { - await client.volumes.read({ name }); + await getResource(client, "/api/2.1/unity-catalog/volumes", name); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); @@ -253,7 +254,7 @@ const probeVectorIndex: ExistenceProbe = async (client, target) => { const name = field(target, "indexName", "index_name", "name"); if (!name) return missingField("indexName"); try { - await client.vectorSearchIndexes.getIndex({ index_name: name }); + await getResource(client, "/api/2.0/vector-search/indexes", name); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); @@ -264,7 +265,7 @@ const probeFunction: ExistenceProbe = async (client, target) => { const name = field(target, "name"); if (!name) return missingField("name"); try { - await client.functions.get({ name }); + await getResource(client, "/api/2.1/unity-catalog/functions", name); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); diff --git a/packages/shared/src/cli/commands/doctor/checks.test.ts b/packages/shared/src/cli/commands/doctor/checks.test.ts index e64b9d41a..d8de04bd7 100644 --- a/packages/shared/src/cli/commands/doctor/checks.test.ts +++ b/packages/shared/src/cli/commands/doctor/checks.test.ts @@ -375,7 +375,7 @@ describe("checkAuth", () => { delete process.env.DATABRICKS_HOST; mockGetServiceClient.mockResolvedValue({ client: { - config: { host: "https://from-profile.cloud.databricks.com" }, + getHost: async () => "https://from-profile.cloud.databricks.com", currentUser: { me: async () => ({ userName: "u" }) }, }, }); @@ -384,33 +384,35 @@ describe("checkAuth", () => { expect(result.host).toBe("https://from-profile.cloud.databricks.com"); }); - it("reads the resolved host only after me(), which is when the SDK resolves", async () => { - // The SDK populates config.host lazily on the first API call, so reading it - // at construction time would always yield undefined. - delete process.env.DATABRICKS_HOST; - const client: { - config: { host?: string }; - currentUser: { me: () => Promise<{ userName: string }> }; - } = { - config: {}, - currentUser: { - me: async () => { - client.config.host = "https://resolved-late.cloud.databricks.com"; - return { userName: "u" }; + it("falls back to DATABRICKS_HOST when host resolution itself fails", async () => { + process.env.DATABRICKS_HOST = "https://env.cloud.databricks.com"; + mockGetServiceClient.mockResolvedValue({ + client: { + getHost: async () => { + throw new Error( + 'profile not found: "prod" in /home/u/.databrickscfg', + ); + }, + currentUser: { + me: async () => { + throw new Error( + 'profile not found: "prod" in /home/u/.databrickscfg', + ); + }, }, }, - }; - mockGetServiceClient.mockResolvedValue({ client }); + }); const { result } = await checkAuth({ profile: "prod" }); - expect(result.host).toBe("https://resolved-late.cloud.databricks.com"); + expect(result.status).toBe("error"); + expect(result.host).toBe("https://env.cloud.databricks.com"); }); it("keeps the resolved host on a failure after the client was built", async () => { delete process.env.DATABRICKS_HOST; mockGetServiceClient.mockResolvedValue({ client: { - config: { host: "https://from-profile.cloud.databricks.com" }, + getHost: async () => "https://from-profile.cloud.databricks.com", currentUser: { me: async () => { throw new Error("boom"); @@ -424,35 +426,24 @@ describe("checkAuth", () => { expect(result.host).toBe("https://from-profile.cloud.databricks.com"); }); - it("recovers the host from the SDK error when no client was built", async () => { - delete process.env.DATABRICKS_HOST; + // sdk-core's profile resolver words this differently from the legacy SDK + // ("profile not found: …" vs "has no … profile configured"). + it("hints an existing --profile when the modular SDK can't find the profile", async () => { mockGetServiceClient.mockRejectedValue( - new Error( - "default auth: cannot configure default credentials. Config: host=https://dbc-abc123.cloud.databricks.com, profile=DEFAULT", - ), + new Error('profile not found: "nope" in /home/u/.databrickscfg'), ); - const { result } = await checkAuth({}); - expect(result.host).toBe("https://dbc-abc123.cloud.databricks.com"); - }); - - it("strips prose punctuation off a host recovered from an error", async () => { - delete process.env.DATABRICKS_HOST; - mockGetServiceClient.mockRejectedValue( - new Error( - "cannot configure default credentials. host=https://foo.cloud.databricks.com.", - ), + const { result } = await checkAuth({ profile: "nope" }); + expect(result.hint).toBe( + "Run `databricks auth login --profile nope`, or pass an existing profile via --profile.", ); - - const { result } = await checkAuth({}); - expect(result.host).toBe("https://foo.cloud.databricks.com"); }); it("never leaks credentials embedded in a resolved host", async () => { delete process.env.DATABRICKS_HOST; mockGetServiceClient.mockResolvedValue({ client: { - config: { host: "https://user:secret@foo.cloud.databricks.com" }, + getHost: async () => "https://user:secret@foo.cloud.databricks.com", currentUser: { me: async () => ({ userName: "u" }) }, }, }); diff --git a/packages/shared/src/cli/commands/doctor/checks.ts b/packages/shared/src/cli/commands/doctor/checks.ts index b61d8f21d..e5ca39b33 100644 --- a/packages/shared/src/cli/commands/doctor/checks.ts +++ b/packages/shared/src/cli/commands/doctor/checks.ts @@ -24,14 +24,13 @@ export interface AuthOutcome { interface CurrentUserClient { currentUser: { - me: () => Promise<{ id?: string; userName?: string }>; + me: (req: object) => Promise<{ id?: string; userName?: string }>; }; } -/** Just the resolved config off a WorkspaceClient, read structurally to keep - * `shared` SDK-free. */ -interface ConfiguredClient { - config?: { host?: unknown }; +/** The host resolver off a WorkspaceClient, read structurally. */ +interface HostClient { + getHost?: () => Promise; } /** @@ -39,12 +38,17 @@ interface ConfiguredClient { * contacted: `DATABRICKS_HOST` wins the host when set, but a profile supplies it * otherwise, so the env var alone can't tell you the workspace in play. * - * Only populated once the SDK has resolved its config, which it does lazily on - * the first API call — reading it right after construction yields undefined. + * Resolved from env + profile only (no network), so it's available even when + * the credentials fail. Undefined when resolution itself fails (e.g. a missing + * profile, or no host anywhere). */ -function resolvedHostOf(client: unknown): string | undefined { - const host = (client as ConfiguredClient | undefined)?.config?.host; - return typeof host === "string" && host.length > 0 ? host : undefined; +async function resolvedHostOf(client: unknown): Promise { + try { + const host = await (client as HostClient | undefined)?.getHost?.(); + return typeof host === "string" && host.length > 0 ? host : undefined; + } catch { + return undefined; + } } /** Compares hosts ignoring scheme, trailing slash, and case, so @@ -80,17 +84,6 @@ async function hostProfileConflict( ); } -/** - * Last-resort host recovery for when the client never got built: the SDK's - * ConfigError appends the resolved config as `host=, profile=…`. - */ -function hostFromError(message: string): string | undefined { - const match = message.match(/host=([^\s,]+)/); - // The config list is prose-terminated ("…databricks.com."), so drop trailing - // punctuation the capture picked up. - return match ? match[1].replace(/[.,]+$/, "") : undefined; -} - /** * Strips URL userinfo (`user:pass@`) from a host, so credentials someone * embedded in `DATABRICKS_HOST` never reach the report or `--json`. Returns the @@ -186,12 +179,10 @@ export async function checkAuth(options: DoctorOptions): Promise { // Bound the live call so an unresponsive workspace can't hang the CLI; a // timeout throws and is reported as an auth failure below. const me = await withTimeout( - (client as CurrentUserClient).currentUser.me(), + (client as CurrentUserClient).currentUser.me({}), ); const who = me.userName ?? me.id ?? "unknown"; - // Read only now: the SDK resolves its config lazily on the first call, so - // before me() the host is still undefined. - const resolvedHost = sanitizeHost(resolvedHostOf(client)) ?? host; + const resolvedHost = sanitizeHost(await resolvedHostOf(client)) ?? host; // Credentials work, but the env/profile split still needs fixing — surface // it as a warning rather than letting a green tick imply all is well. @@ -238,9 +229,8 @@ export async function checkAuth(options: DoctorOptions): Promise { const shownProfile = profile ?? (sdkProfile ? `${sdkProfile} (resolved)` : undefined); // Prefer the host the SDK resolved (covers a profile-supplied host), then - // the one embedded in its error, then the raw env var. - const shownHost = - sanitizeHost(resolvedHostOf(built) ?? hostFromError(raw)) ?? host; + // the raw env var. + const shownHost = sanitizeHost(await resolvedHostOf(built)) ?? host; return { result: { status: "error", @@ -281,7 +271,7 @@ function authFailureHint( return "Check that the workspace host is correct and reachable (verify DATABRICKS_HOST or the profile's host, and that you're online)."; } if ( - /has no .* profile configured|profile .* (does not exist|not found)/i.test( + /has no .* profile configured|profile .*(does not exist|not found)/i.test( message, ) ) { @@ -289,7 +279,7 @@ function authFailureHint( } // Expired/failed token, or no credentials resolved: same next action. if ( - /cannot get access token|refresh token|reauthenticate|databricks auth token|token .*expired|cannot configure default credentials|default auth|no .*credentials/i.test( + /cannot get access token|refresh token|reauthenticate|databricks auth token|token .*expired|cannot configure default credentials|default auth|no .*credentials|host is required/i.test( message, ) ) { diff --git a/packages/shared/src/cli/commands/doctor/databricks-client.ts b/packages/shared/src/cli/commands/doctor/databricks-client.ts index 5c1b563c0..bb915b4f1 100644 --- a/packages/shared/src/cli/commands/doctor/databricks-client.ts +++ b/packages/shared/src/cli/commands/doctor/databricks-client.ts @@ -8,7 +8,8 @@ import { createWorkspaceClient, - loadConfigFile, + resolveProfile, + type WorkspaceClient, } from "../../../workspace-client"; /** @@ -41,14 +42,12 @@ function isModuleNotFound(err: unknown): boolean { } /** Constructs a workspace client via the SDK's unified-auth chain. An explicit - * `profile` is passed through `Config.profile` rather than mutating - * `process.env`, so it doesn't leak beyond this call. */ + * `profile` is passed as a client option rather than mutating `process.env`, so + * it doesn't leak beyond this call. */ export async function getServiceClient( profile?: string, ): Promise { - const client = createWorkspaceClient( - profile ? { profile } : {}, - ).toLegacyWorkspaceClient(); + const client = createWorkspaceClient(profile ? { profile } : {}); return { client }; } @@ -63,10 +62,8 @@ export async function getProfileHost( profile: string, ): Promise { try { - const { iniFile } = await loadConfigFile( - process.env.DATABRICKS_CONFIG_FILE, - ); - const host = iniFile?.[profile]?.host; + // `disableEnv`: only the file's value counts; `DATABRICKS_HOST` already won. + const { host } = await resolveProfile({ profile, disableEnv: true }); return typeof host === "string" && host.length > 0 ? host : undefined; } catch { return undefined; @@ -114,8 +111,15 @@ export async function getLakebasePool( // that dumps the raw SDK ApiError (stack + full response blob) to stderr on a // failed token fetch. doctor classifies and prints that failure itself, so the // library's dump is just noise on top of our clean one-line report. + // `@databricks/lakebase` is still typed against the legacy client: it calls + // `apiClient.request` and an argument-less `currentUser.me()`, which the + // modular SCIM client rejects, so hand it that exact shape. + const ws = client as WorkspaceClient; const pool = appkit.createLakebasePool({ - workspaceClient: client, + workspaceClient: { + apiClient: ws.apiClient, + currentUser: { me: () => ws.currentUser.me({}) }, + }, logger: { error: false }, }); return pool as unknown as LakebasePoolHandle; diff --git a/packages/shared/src/cli/commands/registry/workspace-picker.test.ts b/packages/shared/src/cli/commands/registry/workspace-picker.test.ts index 7153e0d96..819150577 100644 --- a/packages/shared/src/cli/commands/registry/workspace-picker.test.ts +++ b/packages/shared/src/cli/commands/registry/workspace-picker.test.ts @@ -70,7 +70,9 @@ describe("listWorkspaceResources", () => { it("returns choices from a successful warehouse list (SDK)", async () => { const factory = () => fakeClient({ - warehouses: { list: asyncList([{ id: "w1", name: "One" }]) }, + warehouses: { + listWarehousesIter: asyncList([{ id: "w1", name: "One" }]), + }, }); const res = await listWorkspaceResources( "sql_warehouse", @@ -83,10 +85,12 @@ describe("listWorkspaceResources", () => { }); }); - it("maps job_id + settings.name for jobs", async () => { + it("maps jobId (bigint) + settings.name for jobs", async () => { const factory = () => fakeClient({ - jobs: { list: asyncList([{ job_id: 42, settings: { name: "ETL" } }]) }, + jobs: { + listJobsIter: asyncList([{ jobId: 42n, settings: { name: "ETL" } }]), + }, }); const res = await listWorkspaceResources("job", undefined, factory); expect(res.choices).toEqual([{ value: "42", label: "ETL (42)" }]); @@ -96,8 +100,8 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ genie: { - listSpaces: async () => ({ - spaces: [{ space_id: "s1", title: "Sales" }], + genieListSpaces: async () => ({ + spaces: [{ spaceId: "s1", title: "Sales" }], }), }, }); @@ -105,23 +109,21 @@ describe("listWorkspaceResources", () => { expect(res.choices).toEqual([{ value: "s1", label: "Sales (s1)" }]); }); - // genie listSpaces is single-page; the adapter must follow next_page_token + // genie listSpaces is single-page; the adapter must follow nextPageToken // so large workspaces aren't capped at one page. - it("follows genie next_page_token across pages", async () => { - const pages: Record< - string, - { spaces: unknown[]; next_page_token?: string } - > = { - "": { spaces: [{ space_id: "s1" }], next_page_token: "p2" }, - p2: { spaces: [{ space_id: "s2" }] }, - }; + it("follows genie nextPageToken across pages", async () => { + const pages: Record = + { + "": { spaces: [{ spaceId: "s1" }], nextPageToken: "p2" }, + p2: { spaces: [{ spaceId: "s2" }] }, + }; const seen: (string | undefined)[] = []; const factory = () => fakeClient({ genie: { - listSpaces: async (req: { page_token?: string }) => { - seen.push(req.page_token); - return pages[req.page_token ?? ""]; + genieListSpaces: async (req: { pageToken?: string }) => { + seen.push(req.pageToken); + return pages[req.pageToken ?? ""]; }, }, }); @@ -134,9 +136,9 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ genie: { - listSpaces: async () => ({ - spaces: [{ space_id: "s1" }], - next_page_token: "same", + genieListSpaces: async () => ({ + spaces: [{ spaceId: "s1" }], + nextPageToken: "same", }), }, }); @@ -145,9 +147,34 @@ describe("listWorkspaceResources", () => { expect(res.choices.length).toBeGreaterThan(0); }); + // Services without a modular client go through the facade's raw request(). + it("lists REST-backed types via request(), following next_page_token", async () => { + const calls: unknown[] = []; + const pages: Record = { + "": { apps: [{ name: "a1" }], next_page_token: "p2" }, + p2: { apps: [{ name: "a2" }] }, + }; + const factory = () => + fakeClient({ + request: async (req: { + path: string; + query?: { page_token?: string }; + }) => { + calls.push(req); + return Response.json(pages[req.query?.page_token ?? ""]); + }, + }); + const res = await listWorkspaceResources("app", undefined, factory); + expect(res.choices.map((c) => c.value)).toEqual(["a1", "a2"]); + expect(calls).toEqual([ + { method: "GET", path: "/api/2.0/apps", query: undefined }, + { method: "GET", path: "/api/2.0/apps", query: { page_token: "p2" } }, + ]); + }); + it("passes the profile to the client factory", async () => { const factory = vi.fn(() => - fakeClient({ warehouses: { list: asyncList([]) } }), + fakeClient({ warehouses: { listWarehousesIter: asyncList([]) } }), ); await listWorkspaceResources("sql_warehouse", "my-profile", factory); expect(factory).toHaveBeenCalledWith("my-profile"); @@ -159,7 +186,7 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ warehouses: { - list: () => + listWarehousesIter: () => (async function* () { for (let i = 0; i < 10_000; i++) { yielded++; @@ -190,7 +217,7 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ warehouses: { - list: () => { + listWarehousesIter: () => { throw new Error("auth failed"); }, }, @@ -205,7 +232,7 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ warehouses: { - list: () => { + listWarehousesIter: () => { throw new Error( "default auth: cannot configure default credentials", ); diff --git a/packages/shared/src/cli/commands/registry/workspace-picker.ts b/packages/shared/src/cli/commands/registry/workspace-picker.ts index c2e5e9c49..ea98c50a4 100644 --- a/packages/shared/src/cli/commands/registry/workspace-picker.ts +++ b/packages/shared/src/cli/commands/registry/workspace-picker.ts @@ -2,15 +2,15 @@ import { spawnSync } from "node:child_process"; import { createWorkspaceClient, - type LegacyWorkspaceClient, + type WorkspaceClient, } from "../../../workspace-client"; /** * Lists a user's real Databricks workspace resources so `appkit add` can offer * a picker instead of blind free-text entry. * - * Flat resource types are listed via the Databricks SDK client (typed, - * auto-paginating) obtained through the sanctioned `workspace-client` facade. + * Flat resource types are listed through the sanctioned `workspace-client` + * facade: its modular SDK clients where one exists, else a raw REST `GET`. * Parent-context types (volume, uc_function, secret, vector_search_index) still * shell out to the `databricks` CLI for their drill-down. Every path fails * soft: any error returns an empty list and the caller drops to free-text entry. @@ -31,7 +31,7 @@ export interface WorkspaceChoice { * we simply iterate to completion. */ interface SdkLister { - list: (client: LegacyWorkspaceClient) => AsyncIterable; + list: (client: WorkspaceClient) => AsyncIterable; toChoice: (item: Record) => WorkspaceChoice | null; } @@ -53,61 +53,94 @@ function choiceFrom( } /** - * Genie listSpaces returns a single page, not an auto-paginating iterable like - * the other services. Adapt it to one that follows `next_page_token`; the - * repeated-token guard avoids an infinite loop if the API echoes a token back. + * Follows page tokens until exhausted. The repeated-token guard avoids an + * infinite loop if the API echoes a token back. */ -async function* iterateGenieSpaces( - client: LegacyWorkspaceClient, +async function* paginate( + fetchPage: ( + pageToken?: string, + ) => Promise<{ items?: unknown[]; next?: string }>, ): AsyncIterable { let pageToken: string | undefined; do { - const res = await client.genie.listSpaces( - pageToken ? { page_token: pageToken } : {}, - ); - for (const space of res.spaces ?? []) yield space; - const next = res.next_page_token; + const { items, next } = await fetchPage(pageToken); + for (const item of items ?? []) yield item; if (next && next === pageToken) break; pageToken = next; } while (pageToken); } +/** + * Lists a REST collection (snake_case body, `page_token`/`next_page_token`), + * for services the facade has no modular client for yet. + */ +function restList( + client: WorkspaceClient, + path: string, + key: string, +): AsyncIterable { + return paginate(async (pageToken) => { + const res = await client.request({ + method: "GET", + path, + query: pageToken ? { page_token: pageToken } : undefined, + }); + const body = (await res.json()) as Record; + return { + items: body[key] as unknown[] | undefined, + next: body.next_page_token as string | undefined, + }; + }); +} + +/** Genie listSpaces returns a single page; follow `nextPageToken`. */ +function iterateGenieSpaces(client: WorkspaceClient): AsyncIterable { + return paginate(async (pageToken) => { + const res = await client.genie.genieListSpaces( + pageToken ? { pageToken } : {}, + ); + return { items: res.spaces, next: res.nextPageToken }; + }); +} + /** Flat, top-level listable resource types, backed by SDK services. */ export const SDK_LISTERS: Record = { sql_warehouse: { - list: (c) => c.warehouses.list({}), + list: (c) => c.warehouses.listWarehousesIter({}), toChoice: (i) => choiceFrom(i, "id", "name"), }, job: { - list: (c) => c.jobs.list({}), - // job name lives under settings.name; id is top-level job_id + list: (c) => c.jobs.listJobsIter({}), + // job name lives under settings.name; id is top-level jobId (a bigint) toChoice: (i) => { const settings = i.settings as { name?: string } | undefined; - return choiceFrom({ ...i, name: settings?.name }, "job_id", "name"); + return choiceFrom({ ...i, name: settings?.name }, "jobId", "name"); }, }, serving_endpoint: { - list: (c) => c.servingEndpoints.list(), + list: (c) => restList(c, "/api/2.0/serving-endpoints", "endpoints"), toChoice: (i) => choiceFrom(i, "name", "name"), }, uc_connection: { - list: (c) => c.connections.list({}), + list: (c) => + restList(c, "/api/2.1/unity-catalog/connections", "connections"), toChoice: (i) => choiceFrom(i, "name", "full_name"), }, database: { - list: (c) => c.database.listDatabaseInstances({}), + list: (c) => + restList(c, "/api/2.0/database/instances", "database_instances"), toChoice: (i) => choiceFrom(i, "name", "name"), }, genie_space: { list: iterateGenieSpaces, - toChoice: (i) => choiceFrom(i, "space_id", "title"), + toChoice: (i) => choiceFrom(i, "spaceId", "title"), }, experiment: { - list: (c) => c.experiments.listExperiments({}), + list: (c) => restList(c, "/api/2.0/mlflow/experiments/list", "experiments"), toChoice: (i) => choiceFrom(i, "experiment_id", "name"), }, app: { - list: (c) => c.apps.list({}), + list: (c) => restList(c, "/api/2.0/apps", "apps"), toChoice: (i) => choiceFrom(i, "name", "name"), }, }; @@ -118,19 +151,15 @@ export function isFlatListable(resourceType: string): boolean { } /** - * Constructs a raw SDK workspace client for the given profile (or default - * resolution), via the sanctioned `workspace-client` facade. Uses the legacy - * escape hatch because the picker needs services (connections, database, - * experiments, apps) the facade doesn't yet proxy directly. + * Constructs a workspace client for the given profile (or default resolution), + * via the sanctioned `workspace-client` facade. */ -export function makeWorkspaceClient(profile?: string): LegacyWorkspaceClient { - return createWorkspaceClient( - profile ? { profile } : {}, - ).toLegacyWorkspaceClient(); +export function makeWorkspaceClient(profile?: string): WorkspaceClient { + return createWorkspaceClient(profile ? { profile } : {}); } /** - * Max resources fetched for the picker. `list()` auto-paginates, so on a large + * Max resources fetched for the picker. `list()` paginates lazily, so on a large * workspace (5000+ warehouses) breaking out at the cap stops pagination early; * the picker's "Enter manually" option covers anything beyond it. */ @@ -161,9 +190,7 @@ function shortErrorMessage(err: unknown): string { export async function listWorkspaceResources( resourceType: string, profile?: string, - clientFactory: ( - profile?: string, - ) => LegacyWorkspaceClient = makeWorkspaceClient, + clientFactory: (profile?: string) => WorkspaceClient = makeWorkspaceClient, ): Promise { const lister = SDK_LISTERS[resourceType]; if (!lister) return { choices: [], truncated: false }; diff --git a/packages/shared/src/workspace-client/index.ts b/packages/shared/src/workspace-client/index.ts index 74f08cb15..1813c20da 100644 --- a/packages/shared/src/workspace-client/index.ts +++ b/packages/shared/src/workspace-client/index.ts @@ -6,6 +6,7 @@ */ export { ApiError } from "./errors"; export { createWorkspaceClient } from "./factory"; +export { resolveProfile } from "./modular"; export type { CancellationToken, ClientOptions, diff --git a/packages/shared/src/workspace-client/modular.ts b/packages/shared/src/workspace-client/modular.ts index a4b68a77e..e47438c87 100644 --- a/packages/shared/src/workspace-client/modular.ts +++ b/packages/shared/src/workspace-client/modular.ts @@ -393,6 +393,12 @@ export function buildTablesClient(opts: WorkspaceClientOptions): TablesClient { return new TablesClient(mapToClientOptions(opts)); } +/** + * Resolve a `~/.databrickscfg` profile (+ env overlay, unless disabled). Exposed + * so the CLI can read a profile offline without importing the SDK directly. + */ +export { resolve as resolveProfile } from "@databricks/sdk-core/profiles"; + // ── Client type re-exports (for the facade accessor types) ─────────────── export type { FilesClient } from "@databricks/sdk-files/v2"; export type { GenieClient } from "@databricks/sdk-genie/v1";