Skip to content

refactor(bitbucket): centralize Data Center pagination guard - #1105

Open
dpol1 wants to merge 2 commits into
apache:mainfrom
dpol1:refactor/1052-datacenter-pagination-helper
Open

refactor(bitbucket): centralize Data Center pagination guard#1105
dpol1 wants to merge 2 commits into
apache:mainfrom
dpol1:refactor/1052-datacenter-pagination-helper

Conversation

@dpol1

@dpol1 dpol1 commented Aug 18, 2026

Copy link
Copy Markdown
Member

Summary

  • datacenter.py repeated the same non-advancing-pagination guard
    (next_start <= start plus the isLastPage / non-int checks) in six
    paginators, and coverage was uneven: the issue demonstrated that deleting
    the get_repository_restrictions copy left the whole suite green.
  • Hoist the guard into a single _next_page_start() helper, mirroring the
    _validated_next_url() single-helper pattern cloud.py already uses.
    Behaviour is unchanged — the helper preserves the three stop conditions
    exactly as they were.
  • Pin the behaviour twice: unit tests on the helper's stop conditions
    (last page, missing / non-int / repeated / backwards / advancing
    nextPageStart), and a parameterized regression test that drives all six
    paginators against a non-advancing server response — so removing the guard
    wiring from any one of them now fails the suite.

Type of change

  • Python package (tools/*/ with pyproject.toml)

Test plan

  • New paginator pins run green on the pre-refactor code (they photograph
    existing behaviour), and the helper unit tests go red → green across
    the refactor.
  • Full module suite: 127 passed; ruff check, mypy, and the prek
    workspace hooks (ruff / mypy / pytest) all green.
  • Existing regression tests from
    apache/magpie#1047 and
    apache/magpie#766 are
    untouched and still pass.

RFC-AI-0004 compliance

No new network reach, credentials, or mutations; URL construction,
authentication, and response handling are unchanged.

Linked issues

Closes apache/magpie#1052

Notes for reviewers

  • The pre-existing quirk that bool passes the isinstance(next_start, int)
    check is deliberately preserved — this PR changes structure, not
    behaviour.
  • @KatalKavya96 had expressed interest in this issue (and authored the
    existing discussion-paginator pin in
    apache/magpie#766, which this
    change keeps green). Happy to defer or split credit if a PR of theirs is
    in flight — none was visible after 17 days, so this picks the issue up.

Data Center repeated the same nextPageStart guard in six paginators,
while regression coverage was uneven: removing the repository-
restrictions copy still left the suite green. Hoist the guard into
_next_page_start(), mirroring cloud.py's single-helper pattern.

Add focused unit coverage for the helper's stop conditions and a
parameterized regression test that drives all six paginators against
a non-advancing response.
Comment thread tools/bitbucket/tests/test_bitbucket.py Fixed
Comment thread tools/bitbucket/tests/test_bitbucket.py Fixed
CodeQL flagged two parametrize entries that wrapped a callable in a
lambda without binding any extra arguments. Pass the functions
directly; the remaining lambdas bind extra arguments and stay.
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.

refactor(bitbucket): hoist the Data Center non-advancing-pagination guard into one helper

2 participants