Skip to content

Introduce common pagination utility#4732

Merged
pritishpai merged 10 commits into
mainfrom
chore/pagination_utility
Jun 4, 2026
Merged

Introduce common pagination utility#4732
pritishpai merged 10 commits into
mainfrom
chore/pagination_utility

Conversation

@pritishpai

@pritishpai pritishpai commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

Changes

Introduce shared pagination utilities in framework/utils.py and refactor all manual raw-API pagination loops to use them, eliminating duplicated boilerplate and reducing the risk of pagination bugs (like the missing pagination that caused customer issues with account groups listing).

Linked issues

Closes #4731
Related to #4730

Functionality

paginated_fetch_offset -- Generic SCIM-style offset pagination (startIndex/count). Accepts a callable with signature (start_index: int, count: int) -> dict so callers own query construction. Handles page advancement, empty-page termination, and partial-page detection.

paginated_fetch_cursor -- Generic cursor/token-based pagination (page_token/next_page_token). Handles token forwarding and missing-token termination.

Refactored sites

AccountGroupLookup._list_account_groups (workspace_access/groups.py) -- Previously made a single unpaginated API call. Now uses paginated_fetch_offset with deduplication, matching the pagination logic in the SDK's AccountGroupsAPI.list(). This fixes the root cause where not all account groups were fetched for large accounts. Inner fetch function renamed from fetch_page to fetch_account_groups.

feature_store_listing (workspace_access/generic.py) -- Previously had an inline while True / next_page_token loop. Now delegates to paginated_fetch_cursor. Inner fetch function renamed from fetch_page to fetch_feature_tables.

Tests

  • manually tested
  • added unit tests
  • added integration tests
  • verified on staging environment (screenshot attached)

Unit tests added:

  • Multi-page collection test for feature_store_listing (200 + 50 items across 2 pages)
  • Deduplication test for _list_account_groups (duplicate group ID across pages counted once)

@pritishpai pritishpai marked this pull request as ready for review February 27, 2026 20:21
@pritishpai pritishpai requested a review from a team as a code owner February 27, 2026 20:21
@codecov

codecov Bot commented Feb 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.80%. Comparing base (1c197c7) to head (b0e81ab).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4732      +/-   ##
==========================================
+ Coverage   87.77%   87.80%   +0.02%     
==========================================
  Files         123      123              
  Lines       17569    17595      +26     
  Branches     3713     3717       +4     
==========================================
+ Hits        15422    15449      +27     
  Misses       1458     1458              
+ Partials      689      688       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Feb 27, 2026

Copy link
Copy Markdown

✅ 66/66 passed, 10 flaky, 6 skipped, 5h16m53s total

Flaky tests:

  • 🤪 test_clusters[True] (1m59.611s)
  • 🤪 test_prepare_environment_no_groups_selected (5m10.475s)
  • 🤪 test_group_name_change[suffix] (5m54.072s)
  • 🤪 test_reflect_account_groups_on_workspace_warns_skipping_when_a_workspace_group_has_same_name (10m57.157s)
  • 🤪 test_group_name_change[matching] (5m10.008s)
  • 🤪 test_verify_permissions_for_secrets (5m6.572s)
  • 🤪 test_rename_groups (10m33.851s)
  • 🤪 test_group_matching_names[False] (12m50.971s)
  • 🤪 test_delete_ws_groups_should_not_delete_non_reflected_acc_groups (12m45.773s)
  • 🤪 test_group_name_change[prefix] (20m16.934s)

Running from acceptance #9029

@FastLee FastLee 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.

Few requested changes

Comment thread .github/workflows/acceptance.yml Outdated
Comment thread .github/workflows/nightly.yml Outdated
Comment thread src/databricks/labs/ucx/workspace_access/generic.py Outdated
Comment thread src/databricks/labs/ucx/framework/utils.py Outdated
Comment thread src/databricks/labs/ucx/workspace_access/groups.py Outdated
Comment thread tests/unit/framework/test_utils.py
Introduce paginated_fetch_offset (SCIM startIndex/count) and
paginated_fetch_cursor (token-based) helpers in framework/utils.py
to eliminate duplicated pagination boilerplate across the codebase.
Replace inline pagination loop in AccountGroupLookup._list_account_groups
with the shared paginated_fetch_offset utility, adding proper SCIM
offset pagination and deduplication to the previously unpaginated call.
Replace inline cursor-based pagination loop in feature_store_listing
with the shared paginated_fetch_cursor utility.
Rewrite multi-line list comprehension as for loop (R8923), prefix
unused arguments with underscore (W0613), use implicit booleaness
for empty list checks (C1803).
Rename inner fetch functions to be more descriptive (fetch_page ->
fetch_feature_tables, fetch_account_groups). Redesign
paginated_fetch_offset callable signature to accept (start_index, count)
as ints instead of a mutable dict, so callers own query construction.
Add unit tests for feature_store_listing multi-page collection and
_list_account_groups duplicate-ID deduplication to close coverage gap.
@pritishpai pritishpai enabled auto-merge June 4, 2026 19:10
@pritishpai pritishpai requested a review from FastLee June 4, 2026 19:10

@FastLee FastLee 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.

LGTM, I would suggest spliting the push.yml change into a separate PR

@pritishpai

Copy link
Copy Markdown
Contributor Author

LGTM, I would suggest spliting the push.yml change into a separate PR

#4857

@pritishpai pritishpai added this pull request to the merge queue Jun 4, 2026
Merged via the queue into main with commit 22ba51e Jun 4, 2026
9 checks passed
@pritishpai pritishpai deleted the chore/pagination_utility branch June 4, 2026 22:13
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.

Introduce a common pagination utility for raw API calls

2 participants