Skip to content

Conversation

Lokeshchand33
Copy link

@Lokeshchand33 Lokeshchand33 commented Aug 22, 2025

Summary

This PR adds support for Databricks SQL Warehouses APIs by implementing 11 new actions.

Changes

  • 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 .

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

  • New Features
    • Added Databricks SQL Warehouse management actions: create (with name, cluster size, optional auto-stop), delete by ID, edit config, start/stop by ID.
    • Added discovery actions: list warehouses (optional max results) and get warehouse details.
    • Added governance and settings actions: get/set global SQL Warehouse config and get/set warehouse permissions (ACL).
    • Databricks app extended with corresponding API methods for full lifecycle, config, and permissions.

- 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.
Copy link

vercel bot commented Aug 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
pipedream-docs-redirect-do-not-edit Ignored Ignored Aug 23, 2025 6:23am

Copy link
Contributor

coderabbitai bot commented Aug 22, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary of Changes
Databricks app: SQL Warehouse API methods
components/databricks/databricks.app.mjs
Adds methods: createSQLWarehouse, deleteSQLWarehouse, getSQLWarehouse, listSQLWarehouses, editSQLWarehouse, startSQLWarehouse, stopSQLWarehouse, getSQLWarehouseConfig, setSQLWarehouseConfig, getSQLWarehousePermissions, setSQLWarehousePermissions; each delegates to _makeRequest with appropriate HTTP verbs and endpoints.
Actions: Warehouse lifecycle (create/list/get/edit/start/stop/delete)
components/databricks/actions/create-sql-warehouse/create-sql-warehouse.mjs, components/databricks/actions/list-sql-warehouses/list-sql-warehouses.mjs, components/databricks/actions/get-sql-warehouse/get-sql-warehouse.mjs, components/databricks/actions/edit-sql-warehouse/edit-sql-warehouse.mjs, components/databricks/actions/start-sql-warehouse/start-sql-warehouse.mjs, components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs, components/databricks/actions/delete-sql-warehouse/delete-sql-warehouse.mjs
Adds action modules exposing props and run handlers to create (with name, clusterSize, autoStopMinutes), list (with optional maxResults), retrieve by ID, edit by ID (body object), start/stop by ID, and delete by ID. Each action invokes corresponding app method, exports a $summary, and returns the API response (delete returns { success: true }).
Actions: Warehouse global config
components/databricks/actions/get-sql-warehouse-config/get-sql-warehouse-config.mjs, components/databricks/actions/set-sql-warehouse-config/set-sql-warehouse-config.mjs
Adds actions to get and set global SQL Warehouse configuration. set accepts a config object, both call the app methods, export success summaries, and return responses.
Actions: Warehouse permissions
components/databricks/actions/get-sql-warehouse-permissions/get-sql-warehouse-permissions.mjs, components/databricks/actions/set-sql-warehouse-permissions/set-sql-warehouse-permissions.mjs
Adds actions to get and set warehouse permissions by ID. set accepts an array-based ACL (accessControlList) and sends it as access_control_list to the app method; both export summaries and return responses.

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
Loading
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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A twitch of whiskers, pockets full of code,
I spun up warehouses down the data road.
Start, stop, edit, and grant a key—
My little paws deployed them merrily.
Carpets of logs gleam — SQL burrows grow. 🐇✨

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@adolfo-pd adolfo-pd added the User submitted Submitted by a user label Aug 22, 2025
@pipedream-component-development
Copy link
Collaborator

Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified.

@pipedream-component-development
Copy link
Collaborator

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:

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 workspaces

The _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`;
+    },
🧹 Nitpick comments (19)
components/databricks/databricks.app.mjs (1)

158-172: Consider adding “update” permissions (PATCH) support

You 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 a updateSQLWarehousePermissions method and a corresponding action.

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 reference

The 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 errors

Cluster 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.

components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (1)

11-15: Add dropdown options to reduce ID entry errors

Consider providing options() that lists warehouses (id → name) for warehouseId. 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 ID

Same 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 (or page) or an internal paginator, expose it to users, or add a fetchAll 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: Constrain maxResults to Databricks API limits

We’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 add min and max 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 using propDefinitions.warehouseId if available

Currently, databricks.app.mjs does not export a warehouseId propDefinition, so this change is purely optional. Once you’ve added a warehouseId definition under propDefinitions, 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: no warehouseId in components/databricks/databricks.app.mjs propDefinitions block

Apply this diff only after defining warehouseId in databricks.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.

📥 Commits

Reviewing files that changed from the base of the PR and between bd527ae and 97b92e1.

📒 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.

Also applies to: 114-119, 127-133, 135-141


120-126: Confirm edit endpoint semantics (POST /edit vs PATCH)

/sql/warehouses/{id}/edit with POST 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 that POST to /edit is still preferred in your target regions (some APIs have moved to PATCH for partial updates in recent iterations).


143-157: Workspace-level Warehouse Config: ensure required fields are passed

PUT /sql/config/warehouses often requires enable_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 summary

The 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 UX

The action simply calls the app’s method and exports a concise summary. Matches the documented GET /sql/config/warehouses.

components/databricks/actions/stop-sql-warehouse/stop-sql-warehouse.mjs (1)

17-25: LGTM — correct call and user-facing summary

Stop action calls POST /sql/warehouses/{id}/stop appropriately and exports a helpful summary.

components/databricks/actions/delete-sql-warehouse/delete-sql-warehouse.mjs (1)

17-25: LGTM — correct deletion flow

Runs 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
The startSQLWarehouse 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 in components/databricks/databricks.app.mjs, setSQLWarehouseConfig(args = {}) is implemented just after getSQLWarehouseConfig, targeting the correct /sql/config/warehouses endpoint with HTTP PUT, and accepts arbitrary args (including data 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 usage

Both setSQLWarehousePermissions and getSQLWarehousePermissions are defined in components/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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 97b92e1 and 5a697bd.

📒 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 configuration

I’ve confirmed that:

  • The setSQLWarehousePermissions method in components/databricks/databricks.app.mjs invokes HTTP PUT 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.

@Lokeshchand33
Copy link
Author

Branch updated with latest master. Addressed CodeRabbit’s feedback. Ready for maintainer review.

@Lokeshchand33
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
User submitted Submitted by a user
Projects
Status: Ready for PR Review
Development

Successfully merging this pull request may close these issues.

3 participants