Skip to content

Commit ee6aa2b

Browse files
pffigueiredoclaudecursoragent
authored
fix: make warehouseId optional in ServiceContext when no plugin requires it (#91)
* fix: make warehouseId optional when no plugin requires it Adds a `static requires` mechanism to the Plugin class so plugins can declare which shared resources they need (e.g. warehouseId). At startup, createApp() collects requirements from all plugins and passes them to ServiceContext.initialize(), which only resolves warehouseId when a plugin declared it. Apps using only server() no longer need DATABRICKS_WAREHOUSE_ID set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Pedro Figueiredo <klisarkk@gmail.com> * fix: warehouseID should be optional * fix: rename * refactor: better error handling * fix: error message * fix: branch conflicts * feat: use existing logic * fix: dont commit useless changes * test: add some test scenarios * fix: addres PR comments --------- Signed-off-by: Pedro Figueiredo <klisarkk@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent e7fa24b commit ee6aa2b

5 files changed

Lines changed: 90 additions & 12 deletions

File tree

packages/appkit/src/context/execution-context.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { AsyncLocalStorage } from "node:async_hooks";
2+
import { ConfigurationError } from "../errors";
23
import { ServiceContext } from "./service-context";
34
import {
45
type ExecutionContext,
@@ -64,7 +65,14 @@ export function getWorkspaceClient() {
6465
* Get the warehouse ID promise.
6566
*/
6667
export function getWarehouseId(): Promise<string> {
67-
return getExecutionContext().warehouseId;
68+
const ctx = getExecutionContext();
69+
if (!ctx.warehouseId) {
70+
throw ConfigurationError.resourceNotFound(
71+
"Warehouse ID",
72+
"No plugin requires a SQL Warehouse. Add a sql_warehouse resource to your plugin manifest, or set DATABRICKS_WAREHOUSE_ID",
73+
);
74+
}
75+
return ctx.warehouseId;
6876
}
6977

7078
/**

packages/appkit/src/context/service-context.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ export interface ServiceContextState {
2424
client: WorkspaceClient;
2525
/** The service principal's user ID */
2626
serviceUserId: string;
27-
/** Promise that resolves to the warehouse ID */
28-
warehouseId: Promise<string>;
27+
/** Promise that resolves to the warehouse ID (only present when a plugin requires `SQL_WAREHOUSE` resource) */
28+
warehouseId?: Promise<string>;
2929
/** Promise that resolves to the workspace ID */
3030
workspaceId: Promise<string>;
3131
}
@@ -58,10 +58,12 @@ export class ServiceContext {
5858
* Initialize the service context. Should be called once at app startup.
5959
* Safe to call multiple times - will return the same instance.
6060
*
61+
* @param options - Which shared resources to resolve (derived from plugin manifests).
6162
* @param client - Optional pre-configured WorkspaceClient to use instead
6263
* of creating one from environment credentials.
6364
*/
6465
static async initialize(
66+
options?: { warehouseId?: boolean },
6567
client?: WorkspaceClient,
6668
): Promise<ServiceContextState> {
6769
if (ServiceContext.instance) {
@@ -72,7 +74,7 @@ export class ServiceContext {
7274
return ServiceContext.initPromise;
7375
}
7476

75-
ServiceContext.initPromise = ServiceContext.createContext(client);
77+
ServiceContext.initPromise = ServiceContext.createContext(options, client);
7678
ServiceContext.instance = await ServiceContext.initPromise;
7779
return ServiceContext.instance;
7880
}
@@ -153,11 +155,15 @@ export class ServiceContext {
153155
}
154156

155157
private static async createContext(
158+
options?: { warehouseId?: boolean },
156159
client?: WorkspaceClient,
157160
): Promise<ServiceContextState> {
158161
const wsClient = client ?? new WorkspaceClient({}, getClientOptions());
159162

160-
const warehouseId = ServiceContext.getWarehouseId(wsClient);
163+
const warehouseId = options?.warehouseId
164+
? ServiceContext.getWarehouseId(wsClient)
165+
: undefined;
166+
161167
const workspaceId = ServiceContext.getWorkspaceId(wsClient);
162168
const currentUser = await wsClient.currentUser.me();
163169

packages/appkit/src/context/user-context.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ export interface UserContext {
1111
userId: string;
1212
/** The user's name (from request headers) */
1313
userName?: string;
14-
/** Promise that resolves to the warehouse ID (inherited from service context) */
15-
warehouseId: Promise<string>;
14+
/** Promise that resolves to the warehouse ID (inherited from service context, only present when a plugin requires `SQL_WAREHOUSE` resource) */
15+
warehouseId?: Promise<string>;
1616
/** Promise that resolves to the workspace ID (inherited from service context) */
1717
workspaceId: Promise<string>;
1818
/** Flag indicating this is a user context */

packages/appkit/src/core/appkit.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type {
1010
} from "shared";
1111
import { CacheManager } from "../cache";
1212
import { ServiceContext } from "../context";
13-
import { ResourceRegistry } from "../registry";
13+
import { ResourceRegistry, ResourceType } from "../registry";
1414
import type { TelemetryConfig } from "../telemetry";
1515
import { TelemetryManager } from "../telemetry";
1616

@@ -148,14 +148,22 @@ export class AppKit<TPlugins extends InputPluginMap> {
148148
TelemetryManager.initialize(config?.telemetry);
149149
await CacheManager.getInstance(config?.cache);
150150

151-
// Initialize ServiceContext for Databricks client management
152-
// This provides the service principal client and shared resources
153-
await ServiceContext.initialize(config?.client);
154-
155151
const rawPlugins = config.plugins as T;
156152

153+
// Collect manifest resources via registry
157154
const registry = new ResourceRegistry();
158155
registry.collectResources(rawPlugins);
156+
157+
// Derive ServiceContext needs from what manifests declared
158+
const needsWarehouse = registry
159+
.getRequired()
160+
.some((r) => r.type === ResourceType.SQL_WAREHOUSE);
161+
await ServiceContext.initialize(
162+
{ warehouseId: needsWarehouse },
163+
config?.client,
164+
);
165+
166+
// Validate env vars
159167
registry.enforceValidation();
160168

161169
const preparedPlugins = AppKit.preparePlugins(rawPlugins);

packages/appkit/src/core/tests/databricks.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,62 @@ describe("AppKit", () => {
574574
});
575575
});
576576

577+
describe("createApp derives ServiceContext options from registry", () => {
578+
test("should call ServiceContext.initialize with warehouseId: false when no plugin requires sql_warehouse", async () => {
579+
const contextModule = await import("../../context/service-context");
580+
const initSpy = vi.spyOn(contextModule.ServiceContext, "initialize");
581+
const pluginData = [
582+
{ plugin: CoreTestPlugin, config: {}, name: "coreTest" },
583+
];
584+
await createApp({ plugins: pluginData });
585+
expect(initSpy).toHaveBeenCalledWith({ warehouseId: false }, undefined);
586+
initSpy.mockRestore();
587+
});
588+
589+
test("should call ServiceContext.initialize with warehouseId: true when a plugin requires sql_warehouse", async () => {
590+
const PluginWithRequiredResource = class extends CoreTestPlugin {
591+
static manifest: PluginManifest = {
592+
name: "withResource",
593+
displayName: "With Resource",
594+
description: "Plugin with required warehouse",
595+
resources: {
596+
required: [
597+
{
598+
type: ResourceType.SQL_WAREHOUSE,
599+
alias: "wh",
600+
resourceKey: "warehouse",
601+
description: "Warehouse",
602+
permission: "CAN_USE",
603+
fields: { id: { env: "DATABRICKS_WAREHOUSE_ID" } },
604+
},
605+
],
606+
optional: [],
607+
},
608+
};
609+
};
610+
const prevWh = process.env.DATABRICKS_WAREHOUSE_ID;
611+
process.env.DATABRICKS_WAREHOUSE_ID = "wh-123";
612+
try {
613+
const contextModule = await import("../../context/service-context");
614+
const initSpy = vi.spyOn(contextModule.ServiceContext, "initialize");
615+
await createApp({
616+
plugins: [
617+
{
618+
plugin: PluginWithRequiredResource,
619+
config: {},
620+
name: "withResource",
621+
},
622+
],
623+
});
624+
expect(initSpy).toHaveBeenCalledWith({ warehouseId: true }, undefined);
625+
initSpy.mockRestore();
626+
} finally {
627+
if (prevWh !== undefined) process.env.DATABRICKS_WAREHOUSE_ID = prevWh;
628+
else delete process.env.DATABRICKS_WAREHOUSE_ID;
629+
}
630+
});
631+
});
632+
577633
describe("SDK context binding", () => {
578634
test("should bind SDK methods to plugin instance", async () => {
579635
class ContextTestPlugin implements BasePlugin {

0 commit comments

Comments
 (0)