-
Notifications
You must be signed in to change notification settings - Fork 318
Implement Pager resumption even through into_pages #3286
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: main
Are you sure you want to change the base?
Conversation
|
@LarryOsterman this does not yet include the changes to use the struct like in your PR, but will before I mark it ready. I finally got something working - there was a small but annoyingly difficult bug tail - so I wanted to get a draft up. If you want, the tests are worth validating since that is the behavior @RickWinter, @analogrelay, and we decided on internally. If I'm missing a case, please let me know. |
ee819dc to
73cdd5d
Compare
API Change CheckAPIView identified API level changes in this PR and created the following API reviews azure_core |
By design, we didn't support resuming `Pager` (`ItemIterator`) but, upon further discussion, decided we can but will resume from the current page until all items on the current page are processed. This also fixes a problem that, because of the design, we didn't pass through resumption to the `PageIterator` at all - it couldn't even resume as a `PageIterator` for subsequent pages.
73cdd5d to
5687e7a
Compare
Also elevates parsing errors to the caller.
Integrates some changes from Azure#3279 to make merging that PR a little later easier.
|
@jhendrixMSFT @antkmsft regardless of whether @LarryOsterman gets #3279 merged before we release core, we'll still need some emitter changes coinciding with the changes I made to generated code herein. It's all fairly benign. @LarryOsterman do we want to keep the |
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 updates pager implementations across multiple Azure SDK crates to improve context handling and introduce resumable paging capabilities. The changes standardize how context is passed through pagers, change API option types for list operations, and add functionality to resume paging from a continuation token.
Key changes:
- Updated pager callbacks to accept a
Contextparameter directly instead of extracting it from options - Changed
method_optionsfield types fromClientMethodOptionstoPagerOptionsfor list operation options across Azure Storage (blob, queue), Key Vault (keys, secrets, certificates), and Cosmos services - Added
continuation_token()andwith_continuation_token()methods toItemIteratorfor resuming item-level pagination
Reviewed Changes
Copilot reviewed 10 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/core/azure_core/src/http/pager.rs | Core pager implementation changes including new PagerOptions type, context parameter addition to callbacks, continuation token support for ItemIterator, improved state tracking, and extensive test additions |
| sdk/core/azure_core/src/http/poller.rs | Added AsRef<str> constraint to poller next link type parameter and debug logging for operation requests |
| sdk/core/azure_core/CHANGELOG.md | Documented new continuation token features and bug fixes |
| sdk/core/azure_core_test/src/recording.rs | Fixed module reference from typespec_client_core to azure_core |
| sdk/storage/azure_storage_queue/src/generated/models/method_options.rs | Changed method_options type from ClientMethodOptions to PagerOptions and added import |
| sdk/storage/azure_storage_queue/src/generated/clients/queue_service_client.rs | Updated callback signature to accept context parameter and pass options to pager |
| sdk/storage/azure_storage_queue/CHANGELOG.md | Documented breaking change |
| sdk/storage/azure_storage_blob/src/generated/models/method_options.rs | Changed method_options type from ClientMethodOptions to PagerOptions for three list operation option structs |
| sdk/storage/azure_storage_blob/src/generated/clients/*.rs | Updated callbacks to accept context parameter and pass options to pagers |
| sdk/storage/azure_storage_blob/CHANGELOG.md | Documented breaking changes for affected option structs |
| sdk/keyvault/azure_security_keyvault_secrets/src/generated/models/method_options.rs | Changed method_options type to PagerOptions and updated documentation from "method call" to "pager" |
| sdk/keyvault/azure_security_keyvault_secrets/src/generated/clients/secret_client.rs | Updated callbacks to accept context parameter and restructured callback indentation |
| sdk/keyvault/azure_security_keyvault_secrets/CHANGELOG.md | Documented breaking changes |
| sdk/keyvault/azure_security_keyvault_keys/* | Similar changes as secrets - updated types, callbacks, and documentation |
| sdk/keyvault/azure_security_keyvault_certificates/* | Similar changes as keys - updated types, callbacks, and documentation |
| sdk/cosmos/azure_data_cosmos/src/pipeline/mod.rs | Refactored to use PagerOptions and pass context to callback |
By design, we didn't support resuming
Pager(ItemIterator) but, upon further discussion, decided we can but will resume from the current page until all items on the current page are processed.This also fixes a problem that, because of the design, we didn't pass through resumption to the
PageIteratorat all - it couldn't even resume as aPageIteratorfor subsequent pages.