-
Notifications
You must be signed in to change notification settings - Fork 7
APP-5740 : Add support for iterative pagination to User
, Group
, and Workflow
results
#569
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
Conversation
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 introduces support for iterative pagination for User, Group, and Workflow results by transitioning from direct list returns to wrapper response types that expose a “records” (or nested) property for iteration. Key changes include:
- Updating tests to assert on the new “records” (and nested hits) property instead of treating responses as plain lists.
- Modifying client methods for workflows, users, and groups to return specialized response objects (e.g., WorkflowSearchResponse, UserResponse, GroupResponse) that support iterative pagination methods.
- Enhancing model classes (e.g., WorkflowResponse) with iterator and pagination helper methods.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/unit/test_client.py | Updated assertions to use the “records” attribute for pagination. |
tests/integration/test_workflow_client.py | Adjusted workflow test assertions to access nested hits in the new pagination response. |
tests/integration/admin_test.py | Modified tests for groups and users to iterate over the new “records” property. |
pyatlan/model/workflow.py | Introduced a new WorkflowSearchResponse with private attributes and pagination methods. |
pyatlan/model/user.py | Updated user retrieval functionality to return a UserResponse with iterative pagination. |
pyatlan/client/workflow.py | Changed return types for workflow run methods to return WorkflowSearchResponse objects. |
pyatlan/client/user.py | Adjusted user retrieval methods to return UserResponse objects instead of lists. |
pyatlan/client/group.py | Updated group retrieval methods to now return GroupResponse objects. |
pyatlan/client/atlan.py | Updated method signatures and deprecation notices to align with the new pagination model. |
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.
Good progress!
It looks like I had already added iterative pagination to user
and group
in the past, but it seems that proper testing of those methods is missing. For example, GroupClient.get_members
is correctly implemented and returns a UserResponse
, but GroupClient.get_all()
just returns response.records
, which doesn't support pagination since it's embedded in the response classes (e.g: GroupResponse
) 🤔 ✅
I’ve tested the changes manually - all looks good! (few requested changes below)
Requested Changes
-
Add integration tests for the remaining
user
andgroup
methods where pagination has been implemented.
(We can skipworkflow
for now since it may require re-running workflows multiple times, which could add 1–2 minutes to the integration test job - but make sure it's manually tested locally.) -
Pagination test strategy:
-
Use a small page size to ensure multiple API calls are triggered.
-
To determine the small page size dynamically:
-
First, make an API call with
size: 0
to get the total record count. -
Then, calculate page size as:
size = max(1, math.ceil(TOTAL_ASSETS / 5))
-
-
-
Update documentation for all these pagination methods to reflect their usage and any specifics.
Example
:
atlan-python/tests/integration/test_index_search.py
Lines 351 to 365 in 7340bbf
dsl = DSL( | |
query=query, | |
post_filter=Term.with_type_name(value="AtlasGlossaryTerm"), | |
size=0, # to get the total count | |
) | |
request = IndexSearchRequest(dsl=dsl) | |
results = client.asset.search(criteria=request) | |
# Assigning this here to ensure the total assets | |
# remain constant across different test cases | |
TOTAL_ASSETS = results.count | |
# set page_size to divide into ~5 API calls | |
size = max(1, math.ceil(TOTAL_ASSETS / 5)) | |
request.dsl.size = size |
No description provided.