Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support passing context to the pagination class initialization scope #1520

Closed
edgarrmondragon opened this issue Mar 22, 2023 · 6 comments · Fixed by #2529
Closed

feat: Support passing context to the pagination class initialization scope #1520

edgarrmondragon opened this issue Mar 22, 2023 · 6 comments · Fixed by #2529

Comments

@edgarrmondragon
Copy link
Collaborator

Feature scope

Taps (catalog, state, stream maps, etc.)

Description

Some pagination schemes may required access to the sync context object.

The RESTStream.get_new_paginator method is already being called within RESTStream.request_records, which has access to the context object, so it'd be rather easy to update the signature of the former to support passing it.

paginator = self.get_new_paginator()

@edgarrmondragon
Copy link
Collaborator Author

To accomplish this in a backwards compatible way we'd need to do something like

try:
    paginator = self.get_new_paginator(context=context)
except TypeError:
    paginator = self.get_new_paginator()

An even better way to accomplish the purpose of this feature request would be to make the context accessible within any scope within the stream class:

class Stream:
  @final
  def get_context():
    return self.__context

  def get_new_paginator(self):
    context = self.get_context()
    start_dt = self.get_starting_datetime(context)
    return MyPaginator(start_dt)

Copy link

stale bot commented May 8, 2024

This has been marked as stale because it is unassigned, and has not had recent activity. It will be closed after 21 days if no further activity occurs. If this should never go stale, please add the evergreen label, or request that it be added.

@stale stale bot added the stale label May 8, 2024
@edgarrmondragon
Copy link
Collaborator Author

Still relevant

@kinghuang
Copy link

kinghuang commented Jun 8, 2024

I've just run into this myself. I'm implementing a parent-child stream, and need the context from the parent in order to create the paginator in the child.

@edgarrmondragon
Copy link
Collaborator Author

I've just run into this myself. I'm implementing a parent-child stream, and need the context from the parent in order to create the paginator in the child.

PRs are welcome for this feature!

@edgarrmondragon
Copy link
Collaborator Author

Small enough and I found the time to address this: #2529

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants