refactor(cli): route doctor's SDK access through the shared workspace-client facade - #531
Merged
Merged
Conversation
MarioCadenas
force-pushed
the
feat/registry-cli
branch
from
August 13, 2026 10:30
b2816be to
15bb1be
Compare
MarioCadenas
force-pushed
the
feat/doctor-workspace-client
branch
from
August 13, 2026 15:15
2a673ed to
0a07567
Compare
MarioCadenas
changed the base branch from
feat/registry-cli
to
feat/workspace-client-shared
August 13, 2026 15:20
atilafassina
approved these changes
Aug 13, 2026
MarioCadenas
force-pushed
the
feat/doctor-workspace-client
branch
from
August 13, 2026 17:52
0a07567 to
4b7d53d
Compare
…-client facade
Now that the SDK facade lives in packages/shared/src/workspace-client, the doctor
command reaches the SDK through it instead of a dynamic import + noRestrictedImports
suppression:
- getServiceClient uses createWorkspaceClient({profile}).toLegacyWorkspaceClient()
- getProfileHost uses the facade's re-exported loadConfigFile
- drops both `biome-ignore` suppressions and the dynamic import(...) dance
- adds loadConfigFile to the facade's re-exports
SdkNotInstalledError is kept exported (no longer thrown — shared now hard-depends
on the SDK) so the SDK_NOT_INSTALLED diagnostic branch and tests keep compiling.
The Lakebase probe keeps its dynamic @databricks/appkit import (optional peer).
Also removes @databricks/sdk-experimental from packages/appkit/package.json: with
the facade moved to shared, appkit no longer imports the SDK directly. It still
ships to consumers via shared's deps (dist-appkit.ts merges them into the tarball).
Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
MarioCadenas
force-pushed
the
feat/doctor-workspace-client
branch
from
August 14, 2026 07:39
4b7d53d to
b660293
Compare
Contributor
📦 Bundle size reportCompared against
|
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 869 KB (-553 B) | 303 KB (-89 B) |
| Type declarations | 315 KB (+379 B) | 109 KB (+221 B) |
| Source maps | 1.7 MB (+326 B) | 566 KB (+137 B) |
| Other | 11 KB | 3.7 KB |
| Total | 2.9 MB (+152 B) | 982 KB (+269 B) |
Per-entry composition (own code — deps external (as shipped))
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
. |
88 KB (+18 B) | 2.5 KB | 91 KB (+18 B) | external | 288 KB (+48 B) |
./beta |
49 KB (+7 B) | 456 B (-2 B) | 49 KB (+5 B) | external | 143 KB (+48 B) |
./type-generator |
21 KB (+41 B) | 0 B | 21 KB (+41 B) | external | 61 KB (+48 B) |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
. |
index.js |
initial | 84 KB |
. |
utils.js |
initial | 4.0 KB |
. |
remote-tunnel-manager.js |
lazy | 2.5 KB |
./beta |
beta.js |
initial | 33 KB |
./beta |
stream-manager.js |
initial | 5.8 KB |
./beta |
wide-event-emitter.js |
initial | 3.2 KB |
./beta |
databricks.js |
initial | 3.0 KB |
./beta |
configuration.js |
initial | 2.1 KB |
./beta |
service-context.js |
initial | 1.3 KB |
./beta |
client.js |
initial | 434 B |
./beta |
client-options.js |
initial | 219 B |
./beta |
supervisor-api.js |
lazy | 192 B |
./beta |
databricks.js |
lazy | 141 B |
./beta |
index.js |
lazy | 123 B |
./type-generator |
index.js |
initial | 21 KB |
@databricks/appkit-ui
npm tarball (packed): 342 KB (+351 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 390 KB | 130 KB |
| Type declarations | 228 KB (+412 B) | 83 KB (+360 B) |
| Source maps | 753 KB | 248 KB |
| CSS | 16 KB | 3.3 KB |
| Total | 1.4 MB (+412 B) | 465 KB (+360 B) |
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
./js |
5.3 KB | 49 KB | 55 KB | 208 KB | 14 KB |
./js/beta |
20 B | 0 B | 20 B | 0 B | 0 B |
./react |
432 KB | 49 KB | 480 KB | 1.3 MB | 175 KB |
./react/beta |
1.0 KB | 0 B | 1.0 KB | 0 B | 1.9 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
./js |
index.js |
initial | 5.2 KB |
./js |
chunk |
initial | 120 B |
./js |
apache-arrow |
lazy | 49 KB |
./js/beta |
beta.js |
initial | 20 B |
./react |
index.js |
initial | 430 KB |
./react |
tslib |
initial | 2.1 KB |
./react |
apache-arrow |
lazy | 49 KB |
./react/beta |
beta.js |
initial | 1.0 KB |
MarioCadenas
enabled auto-merge (squash)
August 14, 2026 07:42
MarioCadenas
disabled auto-merge
August 14, 2026 07:46
The doctor command now reaches the SDK through the shared workspace-client
facade, so shared's compiled CLI imports ../../../workspace-client/{factory,legacy}.js.
dist-appkit copied shared's dist/cli into the tarball but not dist/workspace-client,
so any appkit CLI invocation (e.g. the generate-types postinstall, which loads all
commands) crashed with ERR_MODULE_NOT_FOUND. Copy dist/workspace-client alongside
the other CLI leaf modules so those relative imports resolve.
Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
Contributor
🤖 AppKit PR bot🔬 Run evalsStart an eval for this PR from the evals-monitor app: Go to Evals Monitor → 📦 Try this PR's app templateScaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh run download 31782698860 -R databricks/appkit -n appkit-template-0.60.0-pr.3f21656-feat-doctor-workspace-client-531 -D appkit-pr-531 \
&& unzip -o "appkit-pr-531/appkit-template-0.60.0-pr.3f21656-feat-doctor-workspace-client-531.zip" -d "appkit-pr-531" \
&& databricks apps init --template "appkit-pr-531"The template pins |
The doctor routing re-exports loadConfigFile from legacy.ts's SDK destructure; add it to legacy.test.ts's vi.mock so module load doesn't throw "No loadConfigFile export is defined on the mock". Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows the workspace-client relocation (in the base
feat/registry-cli): the SDK facade now lives inpackages/shared/src/workspace-client, reachable from bothappkitand the CLI inshared. This routes thedoctorcommand's SDK access through it, removing the dynamic-import workaround it needed whensharedwas SDK-free.Changes
doctor/databricks-client.ts:getServiceClientnow usescreateWorkspaceClient({profile}).toLegacyWorkspaceClient();getProfileHostuses the facade's re-exportedloadConfigFile. Drops bothbiome-ignore lint/style/noRestrictedImportssuppressions and theawait import("@databricks/sdk-experimental")dance.workspace-clientfacade: addsloadConfigFileto the re-exports (legacy.ts+index.ts).packages/appkit/package.json: removes the now-unused direct@databricks/sdk-experimentaldependency. appkit no longer imports the SDK directly (all access is viashared/workspace-client); it still ships to consumers becausedist-appkit.tsmergesshared's deps into the published tarball.Notes
SdkNotInstalledErroris kept exported (no longer thrown —sharednow hard-depends on the SDK) sochecks.ts'sSDK_NOT_INSTALLEDdiagnostic branch and the existing tests keep compiling. That branch is now effectively dead; removing it is a deliberate separate cleanup.getLakebasePool) keeps its dynamic@databricks/appkitimport — that's an optional peershareddoesn't depend on, unchanged.Verification
shared+appkittypecheck cleanBased on
feat/registry-cli; will retarget tomainonce that merges.This pull request and its description were written by Isaac.
This PR was created with GitHub MCP.