-
Notifications
You must be signed in to change notification settings - Fork 20
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: Generator for paged-queries #75
base: master
Are you sure you want to change the base?
Conversation
@mure , Generator concept implementation for query_tables and example usage |
@@ -119,6 +120,31 @@ def query_tables(self, query: models.QueryTablesRequest) -> models.PagedTables: | |||
""" | |||
... | |||
|
|||
def query_tables_generator( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's weird to me that we're using the term _generator
in the function name here. Shouldn't this just be query_tables
and return Generator
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is not the final name, this PR was raised to discuss the aligned way to proceed
_query = query.copy() | ||
_query.continuation_token = None | ||
while True: | ||
paged_tables = self.query_tables(query=_query) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include exponential backoff on this so that we protect users from running into rate limiting when they use this style for large queries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementing the rate limiter at the client class level will automatically cover all methods; hence, additional handling is not required within the generator.
What does this Pull Request accomplish?
Adds generator variant for paged-queries to simplify usage in loops.
Why should this Pull Request be merged?
Simplify the way users invoke paged-queries without the burden of handling
continuation_token
.What testing has been done?
Automated integration tests.