-
Notifications
You must be signed in to change notification settings - Fork 46
feat(pagination): add cursor-based pagination + client APIs #112
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
base: master
Are you sure you want to change the base?
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #112 +/- ##
==========================================
+ Coverage 88.74% 89.48% +0.73%
==========================================
Files 7 8 +1
Lines 622 713 +91
==========================================
+ Hits 552 638 +86
- Misses 70 75 +5
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:
|
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.
Pull Request Overview
This PR adds cursor-based pagination functionality to pycouchdb, implementing both view and Mango query pagination along with client API methods. The main purpose is to provide stable, memory-efficient pagination without the pitfalls of skip-based pagination.
Key changes:
- Adds a new
pagination.pymodule withview_pages()andmango_pages()functions for cursor-based pagination - Extends the Database class with
find(),view_pages(), andmango_pages()methods - Includes comprehensive test coverage for both unit and integration testing
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pycouchdb/pagination.py | Core pagination logic implementing cursor-based pagination for views and Mango queries |
| pycouchdb/client.py | Database class extensions adding pagination methods and Mango query support |
| pycouchdb/types.py | Type aliases for pagination-related types |
| pycouchdb/utils.py | Enhanced UTF-8 decoding with error handling |
| test/test_pagination.py | Unit tests for pagination functions with mocked dependencies |
| test/test_database.py | Unit tests for new Database methods |
| test/integration/test_pagination_integration.py | Comprehensive integration tests with real CouchDB |
| test/integration/test_integration.py | Additional integration tests for pagination functionality |
| docs/source/quickstart.rst | Documentation updates explaining pagination features |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pycouchdb/pagination.py
Outdated
| last_row = rows[page_size - 1] | ||
| startkey = last_row['key'] | ||
| startkey_docid = last_row['id'] | ||
| skip = 1 # Skip the row we used as cursor |
Copilot
AI
Sep 30, 2025
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.
[nitpick] The inline comment explains what skip=1 does, but it would be clearer to explain why this is necessary to avoid duplicate results in cursor-based pagination.
| skip = 1 # Skip the row we used as cursor | |
| skip = 1 # Skip the row used as the cursor to avoid returning it again (prevents duplicate results in cursor-based pagination) |
pycouchdb/pagination.py
Outdated
| def _prepare_mango_data(selector: Dict[str, Any], params: Dict[str, Any]) -> bytes: | ||
| """Prepare Mango query data for HTTP request.""" | ||
| data_dict = { | ||
| 'selector': selector, | ||
| **params | ||
| } | ||
| return utils.force_bytes(json.dumps(data_dict)) |
Copilot
AI
Sep 30, 2025
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.
The _prepare_mango_data function is defined but never used in the code. Consider removing it or adding a comment explaining its intended future use.
| def _prepare_mango_data(selector: Dict[str, Any], params: Dict[str, Any]) -> bytes: | |
| """Prepare Mango query data for HTTP request.""" | |
| data_dict = { | |
| 'selector': selector, | |
| **params | |
| } | |
| return utils.force_bytes(json.dumps(data_dict)) |
pycouchdb/client.py
Outdated
| :param params: Additional query parameters | ||
| :returns: Iterator yielding lists of rows for each page | ||
| .. versionadded: 1.17 |
Copilot
AI
Sep 30, 2025
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.
The docstring format should be .. versionadded:: 1.17 with double colons, not single colon.
| .. versionadded: 1.17 | |
| .. versionadded:: 1.17 |
pycouchdb/client.py
Outdated
| :param params: Additional query parameters | ||
| :returns: Iterator yielding lists of documents for each page | ||
| .. versionadded: 1.17 |
Copilot
AI
Sep 30, 2025
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.
The docstring format should be .. versionadded:: 1.17 with double colons, not single colon.
| .. versionadded: 1.17 | |
| .. versionadded:: 1.17 |
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.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| last_row = rows[page_size - 1] | ||
| startkey = last_row['key'] | ||
| startkey_docid = last_row['id'] | ||
| skip = 1 # Skip the row used as the cursor to avoid returning it again (prevents duplicate results in cursor-based pagination) |
Copilot
AI
Sep 30, 2025
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.
[nitpick] The comment is overly verbose and could be clearer. Consider simplifying to 'Skip cursor row to avoid duplicates'.
| skip = 1 # Skip the row used as the cursor to avoid returning it again (prevents duplicate results in cursor-based pagination) | |
| skip = 1 # Skip cursor row to avoid duplicates |
| startkey = last_row['key'] | ||
| startkey_docid = last_row['id'] |
Copilot
AI
Sep 30, 2025
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.
The code uses magic string keys 'key' and 'id' without validation. Consider adding defensive checks or constants for these keys to improve maintainability.
| # Valid JSON but with invalid UTF-8 sequence in the middle | ||
| self.content = b'{"key": "value", "invalid": "\xff\xfe"}' |
Copilot
AI
Sep 30, 2025
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.
The byte string contains invalid UTF-8 sequences inside a JSON string literal, but this creates invalid JSON, not just invalid UTF-8. The test may not be testing the intended scenario.
| # Valid JSON but with invalid UTF-8 sequence in the middle | |
| self.content = b'{"key": "value", "invalid": "\xff\xfe"}' | |
| # Valid JSON, but the value for "invalid" is a string with bytes that are invalid UTF-8 | |
| self.content = b'{"key": "value", "invalid": "' + b'\xff\xfe'.decode("latin1").encode("utf-8", "surrogateescape") + b'"}' |
| # Valid JSON with invalid UTF-8 that will be replaced with replacement character | ||
| self.content = b'{"key": "value", "invalid": "\xff\xfe\x80"}' |
Copilot
AI
Sep 30, 2025
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.
Similar to the previous test, this creates invalid JSON syntax rather than testing UTF-8 decode errors. The invalid bytes should be outside the JSON string structure.
No description provided.