feat: Make pagination object available in more scopes of REST stream class #3245
Conversation
Reviewer's GuideThis PR refactors the RESTStream pagination implementation by introducing a private Sequence diagram for paginator lifecycle in request_recordssequenceDiagram
participant RESTStream
participant Paginator
participant Metrics
participant HTTPRequest
RESTStream->>RESTStream: enter _paginator_context()
RESTStream->>Paginator: get_new_paginator()
RESTStream->>RESTStream: set self._current_paginator
RESTStream->>Metrics: start http_request_counter
loop while not paginator.finished
RESTStream->>HTTPRequest: prepare_request(context, next_page_token)
RESTStream->>HTTPRequest: decorated_request(prepared_request, context)
RESTStream->>Metrics: increment counter
RESTStream->>RESTStream: update_sync_costs
RESTStream->>RESTStream: parse_response(resp)
RESTStream->>Paginator: advance(resp)
end
RESTStream->>RESTStream: exit _paginator_context(), cleanup self._current_paginator
Class diagram for updated RESTStream pagination lifecycleclassDiagram
class RESTStream {
- _current_paginator: BaseAPIPaginator | None
+ paginator: BaseAPIPaginator
+ _paginator_context(): Iterator[BaseAPIPaginator]
+ request_records(context: Context | None): Iterable[dict]
+ get_new_paginator()
+ get_url_params(context, next_page_token)
}
RESTStream --> BaseAPIPaginator
RESTStream --> SinglePagePaginator
RESTStream : uses _paginator_context for paginator lifecycle
RESTStream : exposes paginator property during request_records
RESTStream : get_url_params can access self.paginator
class BaseAPIPaginator {
+ current_value
+ finished
+ advance(resp)
+ continue_if_empty(resp)
+ page_size
}
class SinglePagePaginator {
+ finished
}
BaseAPIPaginator <|-- SinglePagePaginator
Class diagram for paginator property and context managerclassDiagram
class RESTStream {
+ paginator: BaseAPIPaginator
+ _paginator_context(): Iterator[BaseAPIPaginator]
}
RESTStream --> BaseAPIPaginator
RESTStream : paginator only available during request_records
RESTStream : _paginator_context manages lifecycle
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Add a test for early generator close to ensure
_current_paginatoralways resets even when iteration stops prematurely - Consider isolating paginator state using thread-local or stack-based storage to support concurrent or nested
request_recordscalls without state collisions
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a test for early generator close to ensure `_current_paginator` always resets even when iteration stops prematurely
- Consider isolating paginator state using thread-local or stack-based storage to support concurrent or nested `request_records` calls without state collisionsHelp me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if offset == 0: | ||
| r._content = json.dumps({"data": [{"id": 1}, {"id": 2}]}).encode() | ||
| elif offset == 2: | ||
| r._content = json.dumps({"data": [{"id": 3}, {"id": 4}]}).encode() | ||
| else: | ||
| r._content = json.dumps({"data": []}).encode() |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| if offset == 0: | ||
| r._content = json.dumps({"data": [{"id": 1}]}).encode() | ||
| else: | ||
| # Return empty data to end pagination | ||
| r._content = json.dumps({"data": []}).encode() |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| if offset == 0: | ||
| r._content = json.dumps({"data": [{"id": 1}]}).encode() | ||
| else: | ||
| # Return empty data to end pagination | ||
| r._content = json.dumps({"data": []}).encode() |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| if skip == 0: | ||
| r._content = json.dumps({"data": [{"id": 1}, {"id": 2}]}).encode() | ||
| else: | ||
| r._content = json.dumps({"data": []}).encode() |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| if skip == 0: | ||
| r._content = json.dumps({"data": [{"id": 1}, {"id": 2}]}).encode() | ||
| else: | ||
| r._content = json.dumps({"data": []}).encode() |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| } | ||
|
|
||
| def _request(self, prepared_request, context): # noqa: ARG002 | ||
| r = Response() |
There was a problem hiding this comment.
issue (code-quality): Extract duplicate code into method (extract-duplicate-method)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3245 +/- ##
==========================================
+ Coverage 92.87% 92.89% +0.01%
==========================================
Files 63 63
Lines 5458 5472 +14
Branches 682 683 +1
==========================================
+ Hits 5069 5083 +14
Misses 282 282
Partials 107 107
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
CodSpeed Performance ReportMerging #3245 will not alter performanceComparing Summary
|
Documentation build overview
Show files changed (3 files in total): 📝 3 modified | ➕ 0 added | ➖ 0 deleted
|
Summary
Resolves #1606
This PR makes the pagination object available as an instance attribute (
self.paginator) across all method scopes within the REST stream class, not just withinrequest_records. This eliminates the need to duplicate pagination configuration values across different methods.Key Changes
_current_paginatorwith proper lifecycle managementget_url_paramswith usage examplesBenefits
request_recordscall gets a new paginator instanceUsage Example
Before (with duplication):
After (no duplication):
Testing
🤖 Generated with Claude Code
Summary by Sourcery
Make the pagination object available across all REST stream methods by introducing a context-managed paginator property while preserving existing behaviors
New Features:
Enhancements:
Documentation:
Tests: