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

Fix pagination behaviors for "next_marker" APIs #520

Merged
merged 1 commit into from
Feb 18, 2022

Conversation

sirosen
Copy link
Member

@sirosen sirosen commented Feb 18, 2022

Transfer has several APIs which use a nullable next_marker field to control pagination. In order to support it, make the following changes:

  • MarkerPaginator can be given a marker_key (defaults to "marker")
  • MarkerPaginator has a method used to determine if there is another page of results, defaulting to the previously hardcoded has_next_page check
  • NullableMarkerPaginator(MarkerPaginator) is a new class which alters the has_next_page check to look for a null marker
  • task_skipped_errors, task_successful_transfers, endpoint_manager_task_successful_transfers, and endpoint_manager_task_skipped_errors now all use a NullableMarkerPaginator and set marker_key="next_marker"

Add a test for task_skipped_errors specifically, using generated data, to confirm the behavior of the NullableMarkerPaginator with an altered marker_key.

@sirosen sirosen requested a review from aaschaer February 18, 2022 19:48
Copy link
Contributor

@aaschaer aaschaer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good minus a minor docstring issue

src/globus_sdk/paging/marker.py Outdated Show resolved Hide resolved
Transfer has several APIs which use a nullable `next_marker` field to
control pagination. In order to support it, make the following
changes:

- MarkerPaginator can be given a `marker_key` (defaults to `"marker"`)
- MarkerPaginator has a method used to determine if there is another
  page of results, defaulting to the previously hardcoded
  has_next_page check
- NullableMarkerPaginator(MarkerPaginator) is a new class which alters
  the has_next_page check to look for a null marker
- task_skipped_errors, task_successful_transfers,
  endpoint_manager_task_successful_transfers, and
  endpoint_manager_task_skipped_errors now all use a
  NullableMarkerPaginator and set `marker_key="next_marker"`

Add a test for task_skipped_errors specifically, using generated data,
to confirm the behavior of the NullableMarkerPaginator with an altered
marker_key.
@sirosen sirosen force-pushed the fix-skipped-errors-pagination branch from e107c85 to 79d4492 Compare February 18, 2022 21:07
@sirosen sirosen merged commit 57424b8 into globus:main Feb 18, 2022
@sirosen sirosen deleted the fix-skipped-errors-pagination branch February 18, 2022 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants