Skip to content

fix: migrate from get_next_page_token to get_new_paginator - #603

Open
imamAtif wants to merge 2 commits into
MeltanoLabs:mainfrom
imamAtif:fix/pagination-migration
Open

fix: migrate from get_next_page_token to get_new_paginator#603
imamAtif wants to merge 2 commits into
MeltanoLabs:mainfrom
imamAtif:fix/pagination-migration

Conversation

@imamAtif

@imamAtif imamAtif commented Aug 30, 2026

Copy link
Copy Markdown

Summary

  • replace deprecated RESTStream.get_next_page_token with the new paginator API
  • add GitHubRestPaginator and GitHubGraphQLPaginator in tap_github/client.py
  • update four repository streams (Stargazers, Discussions, DiscussionComments, DiscussionCommentReplies) to override get_new_paginator with has_more early-exit logic instead of get_next_page_token

This removes the deprecation warning:

RESTStream.get_next_page_token is deprecated and will not be used in a future version of the Meltano Singer SDK. Override RESTStream.get_new_paginator instead.

Validation

  • uv run pytest -q - 71 passed, 8 deselected, 2 pre-existing core failures unrelated to this change
  • uv run mypy tap_github - passed
  • uv run ty check tap_github - passed
  • uvx ruff check and uvx ruff format - passed

Closes #171

@imamAtif
imamAtif requested a review from a team as a code owner August 30, 2026 13:12
Replace deprecated RESTStream.get_next_page_token with the new
paginator API. Add GitHubRestPaginator and GitHubGraphQLPaginator
in client.py and update the four repository streams that had
custom early-exit pagination logic to use get_new_paginator with
has_more overrides.

Closes MeltanoLabs#171
@imamAtif
imamAtif force-pushed the fix/pagination-migration branch from afb7c3f to 9d77434 Compare August 30, 2026 13:15

@edgarrmondragon edgarrmondragon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @imamAtif!

Just one request.

Comment thread tap_github/client.py
use_cursor_pagination = False

_authenticator: GitHubTokenAuthenticator | None = None
def __init__(self, stream: GitHubRestStream, *args: Any, **kwargs: Any) -> None: # noqa: ANN401

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd prefer that instead of passing a stream object to the constructor here, we pass the explicit parameters that we get from stream attributes. e.g. self.stream.MAX_RESULTS_LIMIT -> max_results, etc.

Wdyt?

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.

Replace RESTStream.get_next_page_token with RESTStream.get_new_paginator

2 participants