Skip to content

[CXH-2723] - Add Unity Catalog table and volume sync - Databricks Connector - #73

Open
mateoHernandez123 wants to merge 10 commits into
mateoHernandez123/unity-catalog-schema-syncfrom
mateoHernandez123/unity-catalog-table-and-volume-sync
Open

mateoHernandez123 wants to merge 10 commits into
mateoHernandez123/unity-catalog-schema-syncfrom
mateoHernandez123/unity-catalog-table-and-volume-sync

Conversation

@mateoHernandez123

@mateoHernandez123 mateoHernandez123 commented Oct 9, 2026 •

Copy link
Copy Markdown

Description

  • Bug fix
  • New feature

Last of four stacked PRs for Unity Catalog (CXH-2723). Tables and volumes are the data assets an access review is about. They are siblings under a schema.

flowchart TD
  account[account]
  metastore[metastore]
  catalog[catalog]
  schema[schema]
  table[table]
  volume[volume]
  account --> metastore --> catalog --> schema
  schema --> table
  schema --> volume
  style table fill:#1f6feb,color:#fff,stroke:#1f6feb
  style volume fill:#1f6feb,color:#fff,stroke:#1f6feb
Loading

This PR delivers the blue nodes. #70, #71, then #72 merge first.

Sync:

flowchart TD
  schema[schema]
  table[table securable]
  volume[volume]
  schema --> table
  schema --> volume
  table --> managed[MANAGED / EXTERNAL]
  table --> view[VIEW / METRIC_VIEW]
  table --> mv[MATERIALIZED_VIEW]
  table --> st[STREAMING_TABLE]
  table --> empty[missing or unrecognised table_type]
  managed --> tpriv[table privileges]
  view --> vpriv["view set; ALL_PRIVILEGES is not requestable"]
  mv --> mvpriv[view set + REFRESH]
  st --> stpriv["SELECT, READ_METADATA, APPLY_TAG, MANAGE, ALL_PRIVILEGES, REFRESH, MODIFY"]
  empty --> anypriv[union of every variant]
Loading

Provisioning:

  • Grant/Revoke table privileges — the requestable set follows table_type, so ConductorOne does not offer an entitlement the grants API rejects.
    • VIEW / METRIC_VIEW: the view set. ALL_PRIVILEGES is emitted when Databricks already has it and is not requestable. Grant rejects that write before any PATCH. Revoke still removes it.
    • MATERIALIZED_VIEW: the view set plus REFRESH.
    • STREAMING_TABLE: SELECT, READ_METADATA, APPLY_TAG, MANAGE, ALL_PRIVILEGES, REFRESH and MODIFY. Not the table DML trio (DELETE / INSERT / UPDATE).
    • MANAGED / EXTERNAL: the table set, which does not include REFRESH.
    • Missing or unrecognised table_type: union of every variant, so a held REFRESH is not dropped.
  • Grant/Revoke volume privileges — same PATCH path and echo-confirmed idempotency (GrantAlreadyExists / GrantAlreadyRevoked).
  • Owner is read-only. A revoke whose table, volume, schema or catalog Databricks reports as gone (TABLE_DOES_NOT_EXIST, VOLUME_DOES_NOT_EXIST, SCHEMA_DOES_NOT_EXIST, CATALOG_DOES_NOT_EXIST) is GrantAlreadyRevoked. Any other 404 still fails.

Auth:

Unchanged from #71 and #72. A catalog without full grant visibility is skipped, and a schema is listed only when that listing is complete. Tables and volumes are listed through that same workspace. information_schema is skipped.

Architecture highlights:

  • Table listing uses omit_columns and omit_properties. omit_username is not sent: it also drops owner.
  • The tables endpoint rejects max_results above 50; the client clamps.
  • Table and volume Grants use the same principal path as [CXH-2722] - Add Unity Catalog schema sync - Databricks Connector #72: full account SCIM only when assignments exist. An owner-only page uses quoted Find* filters. A 400 on that filter leaves the owner unresolved and does not fail the sync.

Useful links:

@linear-code

linear-code Bot commented Oct 9, 2026

Copy link
Copy Markdown

CXH-2723

@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-schema-sync branch from 920df46 to f9142b4 Compare October 9, 2026 13:20
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-table-and-volume-sync branch from 0ee9040 to 38e403e Compare October 9, 2026 13:20
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-schema-sync branch from f9142b4 to 29274ee Compare October 9, 2026 14:27
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-table-and-volume-sync branch from 38e403e to 7c8e7a3 Compare October 9, 2026 14:27
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-schema-sync branch from 29274ee to e74210e Compare October 9, 2026 16:46
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-table-and-volume-sync branch from 7c8e7a3 to a3bd512 Compare October 9, 2026 16:46
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-schema-sync branch from e74210e to 7deff05 Compare October 9, 2026 16:54
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-table-and-volume-sync branch from a3bd512 to 5dce9b1 Compare October 9, 2026 16:54
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-schema-sync branch from 7deff05 to 1253d7e Compare October 9, 2026 18:17
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-table-and-volume-sync branch from 5dce9b1 to 5effb3f Compare October 9, 2026 18:17

@FeliLucero1 FeliLucero1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a pass on this one. Main things: the per-table perf (the stack-wide one actually bites here), the List/Revoke 404 handling copied from #72, and the table_type mapping dropping REFRESH / ALL_PRIVILEGES grants from the sync. The rest are nits.

Comments tagged (out of scope) are inherited or stack-wide, so treat them as FYI, not blocking for this PR. Build/vet/tests pass locally.

Comment thread pkg/connector/table.go Outdated
Comment thread pkg/connector/table.go Outdated
Comment thread pkg/connector/volume.go Outdated
Comment thread pkg/connector/table.go Outdated
Comment thread pkg/connector/privileges.go Outdated
Comment thread pkg/connector/table.go Outdated
Comment thread pkg/connector/table.go Outdated
Comment thread pkg/connector/table.go Outdated
Comment thread docs/docs-info.md Outdated
Comment thread README.md Outdated

@FeliLucero1 FeliLucero1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image Image Image Image

mateoHernandez123 and others added 2 commits October 9, 2026 16:31
Co-authored-by: Cursor <cursoragent@cursor.com>
…fixes to tables and volumes

Completes the stack: the Revoke pre-read on both securables bypasses the
uhttp response cache so a page cached before the last change cannot make
it skip the PATCH, and a revoke whose catalog is gone reports
GrantAlreadyRevoked rather than failing the task forever.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-schema-sync branch from 1253d7e to 527fd89 Compare October 9, 2026 19:31
@mateoHernandez123
mateoHernandez123 force-pushed the mateoHernandez123/unity-catalog-table-and-volume-sync branch from 5effb3f to 5267eb8 Compare October 9, 2026 19:31
mateoHernandez123 and others added 2 commits October 12, 2026 10:46
…Hernandez123/unity-catalog-table-and-volume-sync

Co-authored-by: Cursor <cursoragent@cursor.com>
…le list 404s

Co-authored-by: Cursor <cursoragent@cursor.com>

@FeliLucero1 FeliLucero1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 2. Thanks for the fixes, most threads look good. I double-checked the List 404: it keeps codes.NotFound through the wrap, so the SDK warns and skips instead of failing the sync. 👍

New: the view ALL_PRIVILEGES entitlement is grantable but always fails (:136), plus a couple of small ones. I replied on the perf, revoke and streaming threads too.

Comment thread pkg/connector/table.go
Comment thread pkg/connector/table.go Outdated
Comment thread pkg/connector/privileges.go Outdated
mateoHernandez123 and others added 2 commits October 12, 2026 15:39
…Hernandez123/unity-catalog-table-and-volume-sync
…olume grants

Co-authored-by: Cursor <cursoragent@cursor.com>
@mateoHernandez123
mateoHernandez123 marked this pull request as draft October 13, 2026 12:32

@sergiocorral-conductorone sergiocorral-conductorone left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review

drafted 2 → confirmed 1, dropped 1, added 0 (verifier: gpt-5.3-codex-high)

Comment thread pkg/connector/privileges.go

@FeliLucero1 FeliLucero1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 3. The perf change looks good overall; one regression in the new owner lookup below. The round-2 comments (view ALL_PRIVILEGES, parent codes, default branch, EqualFold, streaming MODIFY) are still open.

Comment thread pkg/connector/table.go Outdated
… api rejects

Views keep ALL_PRIVILEGES as a reported grant that cannot be requested, an
unrecognised table_type uses the union, and a dropped schema or catalog
revokes as already gone.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mateoHernandez123
mateoHernandez123 marked this pull request as ready for review October 14, 2026 19:13
mateoHernandez123 and others added 3 commits October 14, 2026 16:25
…nd volumes

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants