Skip to content

[CXP-1251] - Promote v0.2.6-test to main - #86

Merged
agustin-conductor merged 1 commit into
mainfrom
bugfix/promote-v0.2.6-test
Oct 29, 2026
Merged

[CXP-1251] - Promote v0.2.6-test to main#86
agustin-conductor merged 1 commit into
mainfrom
bugfix/promote-v0.2.6-test

Conversation

@agustin-conductor

@agustin-conductor agustin-conductor commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Promotes the v0.2.6-test release, which passed validation, to main.

This branch points at the tag's commit (6384cfa1), so the PR head is exactly what was validated. The release holds only #81. staging has later changes (#70 Unity Catalog metastore sync, #56 incremental sync) that weren't part of this release and aren't included.

Replaces #85, which promoted the same change by cherry-picking it.

Change (from #81)

Group and workspace member grants are now paged locally, 2000 per page, using a key cursor. This keeps a single Grants response under the 6MB Lambda payload limit; large groups previously failed with HTTP 413 in sync-grants-for-resource.

🤖 Generated with Claude Code

@agustin-conductor @claude
[CXP-1251] - Paginate group member grants to stay under the Lambda 6M…
6384cfa 
…B response limit (#81)

* Paginate group and workspace member grants to stay under Lambda payload limit

Databricks returns every group member in a single Get group response and has
no member pagination. Grants() emitted one grant per member in one response,
so a group with ~11k+ members (e.g. an all-users account group) exceeded the
6MB Lambda response limit and failed with HTTP 413 on every retry.

Page members locally (GrantsPageSize = 2000, ~1.1MB per page) using the page
token as a member offset, and emit role rule-set grants on the first page
only. Databricks returns members in a different order on every call, so they
are sorted before slicing so pages line up across re-fetches in separate
Lambda invocations. Apply the same paging to workspace assignment grants.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* Page member grants by last emitted key instead of offset

An offset page token into a list that is re-fetched for every page drifts
when members are added or removed between page invocations: a removal skips
the first member of the next page (C1 reads that grant as revoked) and an
addition grants one member twice. Store the key of the last emitted item
(member Ref for groups, principal ID for workspaces) and start the next page
after it, so membership churn no longer shifts page boundaries.

Add churn tests for both paths, a workspace multi-page test, and mark the
test-only shuffle as not security sensitive for gosec.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

---------

Co-authored-by: Claude Opus 5.5 <[email protected]>
@agustin-conductor
agustin-conductor requested a review from a team October 29, 2026 14:56
@linear-code

linear-code Bot commented Oct 29, 2026

Copy link
Copy Markdown

CXP-1251

@github-actions

github-actions Bot commented Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
Superseded — see the current review report for commit 6384cfa15b86

Connector PR Review: [CXP-1251] - Promote v0.2.6-test to main

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base c0a476de267d.
Review mode: full
View review run

Review Summary

This PR brings the #81 change into main, which matches the description. groupBuilder.Grants and workspaceBuilder.Grants still fetch the full member/assignment list in one call, because the API has no member pagination. They now sort that list and page it locally, 2000 per page, using a key cursor (pageAfter / parseCursorToken in pkg/connector/pagination.go). This keeps each response under the 6MB Lambda limit. Group rule-set (manager) grants are only emitted on the first page. I scanned the full PR diff (5 files, no go.mod/go.sum changes) for security and correctness.

I checked these specifically:

  • Cursor behavior: BinarySearchFunc with found → start++ resumes correctly when the cursor item still exists or was removed. If everything after the cursor was removed, it returns an empty last page with no next token.
  • Sort and search agree: the workspace's string keys from strconv.Itoa are sorted lexicographically, and the search uses the same comparison, so they stay consistent.
  • Key uniqueness: member Ref values and principal IDs are unique.
  • Token handling: bag.NextToken("") ends paging.
  • Rate limits: the rate-limit annotations are now returned on the workspace path, which previously returned nil results.
  • Tests: they cover multi-page splitting, reshuffled API ordering, and members added or removed between pages.

How the repo-local criteria applied:

  • S1/S5 (empty results / skipped items): the change adds no new swallowed errors or empty returns. Existing error returns are kept.
  • R8/F2 (pagination): these apply in spirit. Pagination now goes through the SDK bag. Local chunking of an in-memory list is explicitly allowed under C8.
  • L (log levels): the new log lines are Debug only.
  • Not applicable: B (no IDs, slugs or entitlements changed), P/PR (no provisioning code changed), D/J (no response structs changed), T (no spans).

There's no coverage gap.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

@agustin-conductor
agustin-conductor requested review from a team and johnallers October 29, 2026 14:57
@github-actions

github-actions Bot commented Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Connector PR Review: [CXP-1251] - Promote v0.2.6-test to main

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base c0a476de267d.
Review mode: full
View review run

Review Summary

This PR brings the #81 change into main, which matches the description. groupBuilder.Grants and workspaceBuilder.Grants still fetch the full member/assignment list in one call, because the API has no member pagination. They now sort that list and page it locally, 2000 per page, using a key cursor (pageAfter / parseCursorToken in pkg/connector/pagination.go). This keeps each response under the 6MB Lambda limit. Group rule-set (manager) grants are only emitted on the first page. I scanned the full PR diff (5 files, no go.mod/go.sum changes) for security and correctness.

I checked these specifically:

  • Cursor behavior: BinarySearchFunc with found → start++ resumes correctly when the cursor item still exists or was removed. If everything after the cursor was removed, it returns an empty last page with no next token.
  • Sort and search agree: the workspace's string keys from strconv.Itoa are sorted lexicographically, and the search uses the same comparison, so they stay consistent.
  • Key uniqueness: member Ref values and principal IDs are unique.
  • Token handling: bag.NextToken("") ends paging.
  • Rate limits: the rate-limit annotations are now returned on the workspace path, which previously returned nil results.
  • Tests: they cover multi-page splitting, reshuffled API ordering, and members added or removed between pages.

How the repo-local criteria applied:

  • S1/S5 (empty results / skipped items): the change adds no new swallowed errors or empty returns. Existing error returns are kept.
  • R8/F2 (pagination): these apply in spirit. Pagination now goes through the SDK bag. Local chunking of an in-memory list is explicitly allowed under C8.
  • L (log levels): the new log lines are Debug only.
  • Not applicable: B (no IDs, slugs or entitlements changed), P/PR (no provisioning code changed), D/J (no response structs changed), T (no spans).

There's no coverage gap.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Reviewed commit: 6384cfa15b86

github-actions[bot]
github-actions Bot reviewed Oct 29, 2026
View reviewed changes

@github-actions github-actions Bot 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.

No blocking issues found — see the full review report

@agustin-conductor
agustin-conductor merged commit 0c14586 into main Oct 29, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments
@johnallers johnallers johnallers approved these changes
@JavierCarnelli-ConductorOne JavierCarnelli-ConductorOne JavierCarnelli-ConductorOne approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@agustin-conductor @johnallers @JavierCarnelli-ConductorOne