-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Added Databricks SQL Warehouses API actions #18148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Added Databricks SQL Warehouses API actions #18148
Conversation
- Added create SQL warehouse action - Added list SQL warehouses action - Added get SQL warehouse action - Added edit SQL warehouse action - Added delete SQL warehouse action - Added start/stop SQL warehouse actions - Added get/set SQL warehouse config actions - Added get/set SQL warehouse permissions actions Implements 11 SQL Warehouses endpoints as discussed in the enhancement issue.
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
WalkthroughAdds Databricks SQL Warehouse management: app methods for warehouse lifecycle, config, and permissions, plus new action modules that map props to those app methods, call the API, export "$summary" messages, and return responses. Changes
Sequence Diagram(s)sequenceDiagram
actor U as User
participant A as Action (create/get/list/...)
participant D as Databricks App
participant API as Databricks HTTP API
U->>A: Provide props (e.g., name, warehouseId, config)
A->>D: Call method (mapped payload, $)
D->>API: HTTP request to warehouse endpoint
API-->>D: Response
D-->>A: Response
A->>U: Export "$summary" and return response
sequenceDiagram
rect rgba(220,240,255,0.2)
note right of A: Example: List Warehouses
actor U as User
participant A as List Action
participant D as Databricks App
participant API as /sql/config/warehouses/list
U->>A: maxResults (optional)
A->>D: listSQLWarehouses({ params: { limit } , $ })
D->>API: GET /warehouses?limit=...
API-->>D: { warehouses: [...] }
D-->>A: { warehouses }
alt warehouses empty
A->>U: $summary "No warehouses found."
else
A->>U: $summary "Successfully retrieved N warehouse(s)."
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/databricks/databricks.app.mjs (1)
49-51
: Base URL hardcodes AWS host; will fail for Azure and GCP workspacesThe
_baseUrl()
assumes<subdomain>.cloud.databricks.com
, which breaks for:
- Azure:
adb-<ws-id>.<rand>.azuredatabricks.net
- GCP:
<ws-id>.<last-digit>.gcp.databricks.com
Recommend accepting a full host from auth and only falling back to the AWS pattern if a bare instance name is provided.
Apply this diff to make the host flexible across clouds and avoid double-host bugs:
- _baseUrl() { - return `https://${this.$auth.domain}.cloud.databricks.com/api/2.0`; - }, + _baseUrl() { + const domainOrHost = this.$auth.host || this.$auth.domain; + if (!domainOrHost) { + throw new Error("Databricks host/domain is not configured"); + } + const hasScheme = /^https?:\/\//i.test(domainOrHost); + const isFqdn = domainOrHost.includes("."); + // Accept full host (any cloud) if it looks like a URL or FQDN; else treat as AWS short name. + const base = hasScheme + ? domainOrHost.replace(/\/+$/, "") + : (isFqdn ? `https://${domainOrHost}` : `https://${domainOrHost}.cloud.databricks.com`); + return `${base}/api/2.0`; + },
- Azure per-workspace URL format (azuredatabricks.net). (learn.microsoft.com)
- GCP workspace URL format (gcp.databricks.com). (docs.databricks.com, docs.gcp.databricks.com)
🧹 Nitpick comments (19)
components/databricks/databricks.app.mjs (1)
158-172
: Consider adding “update” permissions (PATCH) supportYou added GET/PUT on
/permissions/warehouses/{warehouseId}
. Databricks also supports incremental “update-permissions” (PATCH), which is useful for partial changes without replacing the ACL. Suggest adding aupdateSQLWarehousePermissions
method and a corresponding action.
- CLI includes both set-permissions and update-permissions. (docs.databricks.com)
Possible addition:
+ updateSQLWarehousePermissions({ warehouseId, ...args }) { + return this._makeRequest({ + path: `/permissions/warehouses/${warehouseId}`, + method: "PATCH", + ...args, + }); + },Also, your current permissions path
/permissions/warehouses/{id}
matches Databricks guidance. (kb.databricks.com)components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs (3)
6-6
: Docs URL likely unstable; point to a durable referenceThe link
https://docs.databricks.com/api/workspace/warehouse/create
may redirect or 404 depending on doc versioning. Prefer linking to the SQL Warehouses API or the CLI reference section that consistently lists operations.
- General SQL Warehouses docs and admin settings pages (stable entry points). (docs.databricks.com)
- CLI “warehouses” command group (lists create/edit/start/stop, etc.). (docs.databricks.com)
16-20
: Constrain Cluster Size to known values to prevent API errorsCluster size accepts a fixed set of strings (e.g., 2X-Small, X-Small, Small, Medium, Large, X-Large, 2X-Large, 3X-Large, 4X-Large). Expose these as
options()
to reduce typos.- clusterSize: { - type: "string", - label: "Cluster Size", - description: "Size of the cluster (e.g., `Small`, `Medium`, `Large`)", - }, + clusterSize: { + type: "string", + label: "Cluster Size", + description: "Size of the cluster", + options() { + return [ + "2X-Small", "X-Small", "Small", "Medium", + "Large", "X-Large", "2X-Large", "3X-Large", "4X-Large", + ]; + }, + },Reference size table. (docs.databricks.com)
21-27
: Don’t send unset values; align with workspace defaults
- Defaulting
auto_stop_mins
to 10 may override workspace defaults (UI defaults differ by type). Consider omitting the field when the user leaves it blank.Apply this diff to only include fields when provided, and to support serverless and type selection:
props: { databricks, name: { ... }, clusterSize: { ... }, - autoStopMinutes: { + autoStopMinutes: { type: "integer", label: "Auto Stop (minutes)", description: "Number of minutes of inactivity before the warehouse auto-stops", - optional: true, - default: 10, + optional: true, }, + warehouseType: { + type: "string", + label: "Warehouse Type", + description: "CLASSIC or PRO", + optional: true, + options: ["CLASSIC", "PRO"], + }, + enableServerlessCompute: { + type: "boolean", + label: "Enable Serverless Compute", + description: "When true, set type to PRO and enable serverless (if supported in your region)", + optional: true, + }, }, async run({ $ }) { - const payload = { - name: this.name, - cluster_size: this.clusterSize, - auto_stop_mins: this.autoStopMinutes, - }; + const payload = { + name: this.name, + cluster_size: this.clusterSize, + ...(this.autoStopMinutes != null && { auto_stop_mins: this.autoStopMinutes }), + ...(this.warehouseType && { warehouse_type: this.warehouseType }), + ...(this.enableServerlessCompute != null && { enable_serverless_compute: this.enableServerlessCompute }), + }; + // If serverless is enabled and type not set, default to PRO per Databricks guidance + if (payload.enable_serverless_compute && !payload.warehouse_type) { + payload.warehouse_type = "PRO"; + }Serverless parameter guidance. (docs.databricks.com)
Also applies to: 30-34
components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs (1)
6-6
: Docs link nit“Get SQL Warehouse Config” page locations can vary; consider linking to the warehouse admin settings page (which refers readers to the API) or the CLI
get-workspace-warehouse-config
command reference.
- Admin settings overview and CLI get/set workspace warehouse config. (docs.databricks.com, learn.microsoft.com)
components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (1)
11-15
: Add dropdown options to reduce ID entry errorsConsider providing
options()
that lists warehouses (id → name) forwarehouseId
. This reduces manual ID mistakes and mirrors other Pipedream actions’ UX.warehouseId: { type: "string", label: "Warehouse ID", description: "The ID of the SQL Warehouse to stop", + async options() { + const { warehouses } = await this.databricks.listSQLWarehouses({ $: this }); + return (warehouses || []).map(({ id, name }) => ({ value: id, label: `${name} (${id})` })); + }, },The list endpoint is
/api/2.0/sql/warehouses
. (kb.databricks.com)components/databricks/actions/delete-sql-warehouse/delete-sql-warehouse.mjs (1)
11-15
: Add dropdown options for Warehouse IDSame UX suggestion as “stop”: expose an options list from
listSQLWarehouses
to minimize ID copy/paste errors.warehouseId: { type: "string", label: "Warehouse ID", description: "The ID of the SQL Warehouse to delete", + async options() { + const { warehouses } = await this.databricks.listSQLWarehouses({ $: this }); + return (warehouses || []).map(({ id, name }) => ({ value: id, label: `${name} (${id})` })); + }, },components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs (1)
9-16
: Consider propDefinition with autocomplete for Warehouse ID.Reusing a shared propDefinition improves UX (autocomplete) and consistency across actions.
Apply this diff if your app exposes a reusable definition:
props: { - databricks, - warehouseId: { - type: "string", - label: "Warehouse ID", - description: "The ID of the SQL Warehouse to start", - }, + databricks, + warehouseId: { + propDefinition: [ + databricks, + "warehouseId", + ], + description: "The ID of the SQL Warehouse to start", + }, },components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (2)
16-20
: Guard against empty edits.Calling the API with an empty body will likely 400. Add a quick guard to fail fast with a useful message.
async run({ $ }) { + if (!this.body || Object.keys(this.body).length === 0) { + throw new Error("Please provide at least one field in 'Edit Configuration' to update the warehouse."); + } const response = await this.databricks.editSQLWarehouse({ warehouseId: this.warehouseId, data: this.body, $, });
11-15
: Adopt shared Warehouse ID prop for consistency and UX.Same suggestion as other actions: leverage a
propDefinition
with autocomplete, if available.- warehouseId: { - type: "string", - label: "Warehouse ID", - description: "The ID of the SQL Warehouse to edit", - }, + warehouseId: { + propDefinition: [databricks, "warehouseId"], + description: "The ID of the SQL Warehouse to edit", + },components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (1)
23-24
: Enrich the summary with the warehouse name when available.Minor UX improvement so summaries are identifiable at a glance.
- $.export("$summary", `Retrieved details for SQL Warehouse ID ${this.warehouseId}`); + const name = response?.name || this.warehouseId; + $.export("$summary", `Retrieved details for SQL Warehouse ${name}`);components/databricks/actions/list-sql-warehouses/list-sql-warehouses.mjs (3)
20-22
: Prefer nullish coalescing over||
to respect explicit 0.If 0 is ever valid,
||
would override it.??
handles only null/undefined.- const params = { - limit: this.maxResults || 50, - }; + const params = { + limit: this.maxResults ?? 50, + };
19-37
: Consider pagination options (page/offset or fetch-all).Right now, only the first page is returned. If the app supports
offset
(orpage
) or an internal paginator, expose it to users, or add afetchAll
toggle to iterate and accumulate results.Example (if the app honors
offset
):- async run({ $ }) { - const params = { - limit: this.maxResults ?? 50, - }; + props: { + databricks, + maxResults: { /* …as above… */ }, + page: { + type: "integer", + label: "Page", + description: "0-based page index (translates to offset = page * limit).", + optional: true, + default: 0, + min: 0, + }, + }, + async run({ $ }) { + const params = { + limit: this.maxResults ?? 50, + offset: (this.page ?? 0) * (this.maxResults ?? 50), + };Or, if you prefer
fetchAll: boolean
, I can sketch a loop that collects all pages until an empty page is returned.
11-17
: ConstrainmaxResults
to Databricks API limitsWe’ve confirmed that the Databricks REST API caps the
limit
(i.e.maxResults
) at 100 per request. To prevent client-side errors and align with the API’s behavior, please addmin
andmax
properties:• File:
components/databricks/actions/list-sql-warehouses/list-sql-warehouses.mjs
(around lines 11–17)maxResults: { type: "integer", label: "Max Results", description: "Maximum number of warehouses to return", default: 50, optional: true, + min: 1, + max: 100, // Matches the API’s hard cap per-page limit },This ensures callers can’t request fewer than 1 or more than the API allows (100), avoiding unnecessary round-trip errors and relying on built-in pagination for additional pages.
components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs (2)
11-16
: Add a default and clarify expectations for the config object.A default avoids nullish input in UI and clarifies that a non-empty object is required.
Apply this diff:
config: { type: "object", label: "Configuration", - description: "The configuration object for SQL Warehouses. Example: `{ \"enable_serverless_compute\": true }`", + description: "The configuration object for SQL Warehouses. This may overwrite prior global settings. Example: `{ \"enable_serverless_compute\": true }`", + default: {}, },
17-25
: Guard against empty updates and emit a more informative summary.Prevents accidental no-op (or unintended overwrite if the API treats empty objects specially) and surfaces which keys were updated.
Apply this diff:
async run({ $ }) { - const response = await this.databricks.setSQLWarehouseConfig({ - data: this.config, - $, - }); - - $.export("$summary", "Successfully updated SQL Warehouse configuration"); - return response; + if (!this.config || Object.keys(this.config).length === 0) { + throw new Error("Configuration cannot be empty. Provide at least one field."); + } + const response = await this.databricks.setSQLWarehouseConfig({ + data: this.config, + $, + }); + + const keys = Object.keys(this.config); + $.export( + "$summary", + `Updated SQL Warehouse config (${keys.length} field${keys.length === 1 ? "" : "s"}): ${keys.slice(0, 5).join(", ")}${keys.length > 5 ? ", …" : ""}` + ); + return response; },components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (3)
16-21
: Validate ACL shape and set clear expectations.“Set” likely overwrites the entire ACL. Add a default skeleton and caution in the description to reduce destructive misconfigurations.
Apply this diff:
permissions: { type: "object", label: "Permissions", - description: "The permissions object. Example: `{ \"access_control_list\": [{ \"user_name\": \"alice@example.com\", \"permission_level\": \"CAN_MANAGE\" }] }`", + description: "The permissions object to set (overwrites existing ACL). Example: `{ \"access_control_list\": [{ \"user_name\": \"alice@example.com\", \"permission_level\": \"CAN_MANAGE\" }] }`", + default: { access_control_list: [] }, },
22-31
: Add safety and a more informative summary; prevent accidental ACL replacement.Require explicit confirmation before overwriting a non-empty ACL and surface how many ACEs were applied. This mirrors common “set permissions” guardrails.
Apply this diff:
export default { key: "databricks-set-sql-warehouse-permissions", name: "Set SQL Warehouse Permissions", @@ props: { databricks, warehouseId: { type: "string", label: "Warehouse ID", description: "The ID of the SQL Warehouse to update permissions for", }, permissions: { type: "object", label: "Permissions", description: "The permissions object. Example: `{ \"access_control_list\": [{ \"user_name\": \"alice@example.com\", \"permission_level\": \"CAN_MANAGE\" }] }`", }, + confirmOverwrite: { + type: "boolean", + label: "Confirm overwrite", + description: "Set to true to replace existing permissions using the 'set' API. If false and the target ACL is non-empty, the action aborts.", + optional: true, + default: false, + }, }, async run({ $ }) { - const response = await this.databricks.setSQLWarehousePermissions({ + const acl = this.permissions?.access_control_list; + if (!Array.isArray(acl) || acl.length === 0) { + throw new Error("permissions.access_control_list must be a non-empty array."); + } + + if (!this.confirmOverwrite) { + const existing = await this.databricks.getSQLWarehousePermissions({ + warehouseId: this.warehouseId, + $, + }); + const existingCount = Array.isArray(existing?.access_control_list) ? existing.access_control_list.length : 0; + if (existingCount > 0) { + throw new Error("Existing permissions detected. To overwrite, set 'Confirm overwrite' to true."); + } + } + + const response = await this.databricks.setSQLWarehousePermissions({ warehouseId: this.warehouseId, data: this.permissions, $, }); - $.export("$summary", `Successfully updated permissions for SQL Warehouse ID ${this.warehouseId}`); + const count = acl.length; + $.export("$summary", `Updated permissions for SQL Warehouse ${this.warehouseId} (${count} ACE${count === 1 ? "" : "s"})`); return response; },
11-15
: Suggest usingpropDefinitions.warehouseId
if availableCurrently,
databricks.app.mjs
does not export awarehouseId
propDefinition, so this change is purely optional. Once you’ve added awarehouseId
definition underpropDefinitions
, you can improve UX by letting users select from a dropdown instead of free-text input.• Location:
components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs
lines 11–15
• Note: nowarehouseId
incomponents/databricks/databricks.app.mjs
propDefinitions blockApply this diff only after defining
warehouseId
indatabricks.app.mjs
:- warehouseId: { - type: "string", - label: "Warehouse ID", - description: "The ID of the SQL Warehouse to update permissions for", - }, + // If available in databricks.app.mjs + warehouseId: databricks.propDefinitions?.warehouseId ?? { + type: "string", + label: "Warehouse ID", + description: "The ID of the SQL Warehouse to update permissions for", + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs
(1 hunks)components/databricks/actions/delete-sql-warehouse/delete-sql-warehouse.mjs
(1 hunks)components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs
(1 hunks)components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs
(1 hunks)components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs
(1 hunks)components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs
(1 hunks)components/databricks/actions/list-sql-warehouses/list-sql-warehouses.mjs
(1 hunks)components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs
(1 hunks)components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs
(1 hunks)components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs
(1 hunks)components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs
(1 hunks)components/databricks/databricks.app.mjs
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (2)
components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs (1)
response
(18-21)
components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (3)
components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs (1)
response
(36-39)components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (1)
response
(23-27)components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (1)
response
(18-21)
components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (8)
components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs (1)
response
(36-39)components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs (1)
response
(13-13)components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (1)
response
(18-21)components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (1)
response
(23-27)components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (1)
response
(18-21)
components/databricks/actions/list-sql-warehouses/list-sql-warehouses.mjs (1)
components/databricks/databricks.app.mjs (1)
params
(29-32)
components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs (8)
components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (1)
response
(23-27)components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs (1)
response
(13-13)components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (1)
response
(18-21)components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (1)
response
(23-27)components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (1)
response
(18-21)
components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs (3)
components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs (1)
response
(36-39)components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (1)
response
(23-27)components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs (1)
response
(13-13)
components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs (3)
components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (1)
response
(18-21)components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs (1)
response
(18-21)
components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (7)
components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (1)
response
(23-27)components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs (1)
response
(13-13)components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (1)
response
(23-27)components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (1)
response
(18-21)
components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs (8)
components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs (1)
response
(36-39)components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (1)
response
(23-27)components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs (1)
response
(13-13)components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (1)
response
(18-21)components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (1)
response
(23-27)components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (1)
response
(18-21)
components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (2)
components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (1)
response
(18-21)components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (1)
response
(23-27)
🔇 Additional comments (15)
components/databricks/databricks.app.mjs (3)
93-99
: SQL Warehouses endpoints and methods look correct
- Create/List: POST/GET /api/2.0/sql/warehouses
- Start/Stop: POST /api/2.0/sql/warehouses/{id}/start and /stop
These align with Databricks REST API behavior.
- Start/Stop endpoints reference. (learn.microsoft.com)
Also applies to: 114-119, 127-133, 135-141
120-126
: Confirm edit endpoint semantics (POST /edit vs PATCH)
/sql/warehouses/{id}/edit
withPOST
is consistent with the long-standing pattern, but official pages rarely spell out the HTTP verb. The current Databricks CLI exposes a “warehouses edit” command, which indicates the endpoint exists; please double-check thatPOST
to/edit
is still preferred in your target regions (some APIs have moved to PATCH for partial updates in recent iterations).
- CLI “warehouses edit” command. (learn.microsoft.com)
143-157
: Workspace-level Warehouse Config: ensure required fields are passedPUT
/sql/config/warehouses
often requiresenable_serverless_compute
in the request body; omitting it can produce 400s. Consider documenting this in the action and validating the payload (or providing a sane default) to prevent user errors.Example server response requiring
enable_serverless_compute
. (github.com)components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs (1)
36-43
: LGTM — action wiring and summaryThe request wiring to
databricks.createSQLWarehouse
and the exported$summary
look good.components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs (1)
13-16
: LGTM — correct endpoint usage and clean UXThe action simply calls the app’s method and exports a concise summary. Matches the documented GET
/sql/config/warehouses
.
- GET config reference via CLI docs; same path used in SDK/issue logs. (learn.microsoft.com, github.com)
components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (1)
17-25
: LGTM — correct call and user-facing summaryStop action calls POST
/sql/warehouses/{id}/stop
appropriately and exports a helpful summary.
- Stop endpoint reference. (learn.microsoft.com)
components/databricks/actions/delete-sql-warehouse/delete-sql-warehouse.mjs (1)
17-25
: LGTM — correct deletion flowRuns
DELETE /sql/warehouses/{id}
and returns a simple success payload; summary message is clear.components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs (1)
17-25
: Confirmed startSQLWarehouse implementation
ThestartSQLWarehouse
method is defined in components/databricks/databricks.app.mjs with the correct casing and signature ({ warehouseId, …args }
), matching the action’s usage. No changes required.components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (1)
22-27
: LGTM on the core call.The request assembly looks correct, passing
warehouseId
,data
, and$
to the app method.components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs (1)
17-25
: LGTM on request/response handling.The call signature and summary export follow established action patterns.
components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (1)
17-25
: LGTM on the permissions retrieval.The request shape is consistent with other actions and exports a clear summary.
components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs (2)
3-8
: Solid action scaffold and consistent naming.Key, name, version, and type align with existing actions. Description link is helpful.
1-25
: No changes needed—setSQLWarehouseConfig
exists and matches the contract.
Verified that incomponents/databricks/databricks.app.mjs
,setSQLWarehouseConfig(args = {})
is implemented just aftergetSQLWarehouseConfig
, targeting the correct/sql/config/warehouses
endpoint with HTTP PUT, and accepts arbitraryargs
(includingdata
and$
) as intended.components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (2)
3-8
: Consistent action wiring and metadata.Structure matches the rest of the new actions; versioning and naming are clear.
22-27
: Everything checks out: method contracts match usageBoth
setSQLWarehousePermissions
andgetSQLWarehousePermissions
are defined incomponents/databricks/databricks.app.mjs
with signatures that accept{ warehouseId, ...args }
, and they correctly map to the REST endpoints/permissions/warehouses/${warehouseId}
using PUT and GET respectively, so the action’s usage of{ warehouseId, data, $ }
and{ warehouseId, $ }
is valid.
components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs
Outdated
Show resolved
Hide resolved
components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs
Outdated
Show resolved
Hide resolved
components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs
Outdated
Show resolved
Hide resolved
components/databricks/actions/list-sql-warehouses/list-sql-warehouses.mjs
Outdated
Show resolved
Hide resolved
components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs
Outdated
Show resolved
Hide resolved
components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs
Outdated
Show resolved
Hide resolved
components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (1)
6-6
: Fix docs URL and clarify “set” semantics (replaces entire ACL).The link likely 404s or points to a non-canonical page. Also, “set” replaces all direct ACL entries; this deserves an explicit warning in the description to prevent accidental revocations.
Apply this diff:
- description: "Updates the permissions for a specific SQL Warehouse. [See docs](https://docs.databricks.com/api/workspace/warehouses/setpermissions)", + description: "Sets (replaces) the direct ACL for a specific SQL Warehouse. Principals omitted from the list will be revoked. [See docs](https://docs.databricks.com/api/workspace/permissions)",To double-check the precise deep link for SQL Warehouses:
Please provide the canonical Databricks REST Permissions API docs URL for setting permissions on a SQL Warehouse (the operation that replaces the access_control_list). Also confirm the accepted identity keys and the current list of valid permission levels as of August 2025.
🧹 Nitpick comments (5)
components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (5)
12-15
: Consider using a propDefinition or async options for Warehouse ID.For better UX and fewer copy/paste errors, prefer a dropdown populated from the API (when available in databricks.app.mjs).
Proposed change (only if the app exposes this prop definition):
- warehouseId: { - type: "string", - label: "Warehouse ID", - description: "The ID of the SQL Warehouse to update permissions for", - }, + warehouseId: { + propDefinition: [databricks, "warehouseId"], + description: "The ID of the SQL Warehouse to update permissions for", + },If not available, consider adding an async options loader to the app to list SQL Warehouses and reuse it here.
16-29
: Enforce “exactly one identity” per ACL entry at runtime.The UI description calls this out, but there’s no guard in run(). Add a small validation to fail fast with actionable errors instead of a 400 from the API.
Apply this diff:
async run({ $ }) { - const response = await this.databricks.setSQLWarehousePermissions({ - warehouseId: this.warehouseId, - data: { - access_control_list: this.accessControlList, - }, - $, - }); + // Validate ACL entries: exactly one identity field + permission_level + const entries = this.accessControlList ?? []; + const identityKeys = ["user_name", "group_name", "service_principal_name"]; + const errors = []; + entries.forEach((e, idx) => { + const present = identityKeys.filter((k) => e?.[k]); + if (present.length !== 1) { + errors.push(`Entry ${idx + 1}: expected exactly one of ${identityKeys.join(" | ")}, got ${present.length ? present.join(", ") : "none"}`); + } + if (!e?.permission_level) { + errors.push(`Entry ${idx + 1}: missing permission_level`); + } + }); + if (errors.length) { + throw new Error(`Invalid accessControlList:\n- ${errors.join("\n- ")}`); + } + + const response = await this.databricks.setSQLWarehousePermissions({ + warehouseId: this.warehouseId, + data: { + access_control_list: entries, + }, + $, + });
24-27
: Verify permission level enums; consider avoiding over-restriction in UI.Permission levels for SQL Warehouses vary across editions/regions and over time. If “CAN_MONITOR” isn’t valid for Warehouses in some workspaces, hardcoding it may block users.
Options:
- Option A (safest): don’t constrain with options.
- permission_level: { - type: "string", - options: ["CAN_VIEW", "CAN_MONITOR", "CAN_USE", "CAN_MANAGE"] - }, + permission_level: { + type: "string", + description: "Permission level for this principal (e.g. CAN_VIEW, CAN_USE, CAN_MANAGE).", + },
- Option B (keep enums minimal until confirmed): drop CAN_MONITOR pending docs confirmation.
- options: ["CAN_VIEW", "CAN_MONITOR", "CAN_USE", "CAN_MANAGE"] + options: ["CAN_VIEW", "CAN_USE", "CAN_MANAGE"]If the app layer exposes a helper to fetch permission levels for a given resource type, consider dynamic options instead.
31-41
: Improve the summary to reflect “set” semantics and entry count.Useful feedback to users, especially given set = replace behavior.
Apply this diff:
- $.export("$summary", `Successfully updated permissions for SQL Warehouse ID ${this.warehouseId}`); + $.export("$summary", `Set ${this.accessControlList?.length ?? 0} ACL entr${(this.accessControlList?.length ?? 0) === 1 ? "y" : "ies"} for SQL Warehouse ${this.warehouseId} (replaced existing direct ACL).`);
1-43
: Overall: solid action; minor polish + safeguards recommended.Functionally complete and consistent with the other actions in this PR. The main tweaks are docs link, a runtime ACL validator, and a clearer summary. The rest are optional UX refinements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs
(1 hunks)components/databricks/actions/delete-sql-warehouse/delete-sql-warehouse.mjs
(1 hunks)components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs
(1 hunks)components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs
(1 hunks)components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs
(1 hunks)components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs
(1 hunks)components/databricks/actions/list-sql-warehouses/list-sql-warehouses.mjs
(1 hunks)components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs
(1 hunks)components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs
(1 hunks)components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs
(1 hunks)components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs
🚧 Files skipped from review as they are similar to previous changes (9)
- components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs
- components/databricks/actions/list-sql-warehouses/list-sql-warehouses.mjs
- components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs
- components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs
- components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs
- components/databricks/actions/delete-sql-warehouse/delete-sql-warehouse.mjs
- components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs
- components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs
- components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (2)
components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs (1)
response
(23-27)components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs (1)
response
(18-21)
🔇 Additional comments (2)
components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs (2)
4-8
: Metadata looks good (key/name/type/version).Naming follows the Pipedream convention, action type is correct, and semantic version starts at 0.0.1. No blockers here.
31-38
: Verified Permissions API configurationI’ve confirmed that:
- The
setSQLWarehousePermissions
method incomponents/databricks/databricks.app.mjs
invokes HTTPPUT
on the permissions endpoint:
- Path:
/permissions/warehouses/${warehouseId}
- Method:
PUT
- This aligns with the underlying Databricks Permissions API “set” operation for SQL Warehouses (which is a
PUT
at/api/2.0/permissions/warehouses/{warehouseId}
).No changes needed—everything matches the expected HTTP verb and resource path.
Branch updated with latest master. Addressed CodeRabbit’s feedback. Ready for maintainer review. |
Hi @dylburger , I’ve updated the branch with the latest master and addressed CodeRabbit’s feedback. The PR is ready for review whenever you get a chance. Thanks! |
Summary
This PR adds support for Databricks SQL Warehouses APIs by implementing 11 new actions.
Changes
Implements 11 SQL Warehouses endpoints as discussed in the enhancement issue .
WHY
These additions provide full coverage of SQL Warehouse management in Databricks through Pipedream actions, enabling users to automate provisioning, configuration, lifecycle management, and permissions of SQL Warehouses.
Next Steps
Ready to work on additional sections like Vector Search or MLflow APIs once this is reviewed.
Contributes to #18126
Summary by CodeRabbit