Skip to content

Conversation

@heaths
Copy link
Member

@heaths heaths commented Nov 22, 2025

Resolves #3294

@github-actions github-actions bot added the Azure.Core The azure_core crate label Nov 22, 2025
@heaths
Copy link
Member Author

heaths commented Nov 25, 2025

This is a draft to get some earlier feedback. For now I only changed PageIterator since it was simpler - ItemIterator needs to do some extra things - and because one or the other pager has to change first to store the function instead of the stream. PageIterator is the "leaf" in this scenario, so into_pages() and any tests that rely on it were commented out. Module tests were updated, but no examples or other crates' tests have been updated yet.

I'll likely copy the code to ItemIterator as well and get rid of its from_callback since the type declaration will be complex. I'll define a pager::from_callback or something that will return an impl <Trait> where I want to keep the ItemIterator and PageIterator as trait names. Unfortunately, that also means getting rid of Pager because types can't alias traits currently, not without being dyn; though, to just get the stream maybe that's not so bad?

I'll also copy the implementation to Poller. While I will try to find a way to refactor the shared code for pagers, I probably won't for Poller because there's enough difference that it's probably not worth it and will likely have to diverge in the future anyway. Over-abstracting likely isn't worth it.

@heaths
Copy link
Member Author

heaths commented Nov 26, 2025

I can't make this work. The previous BoxedStream solution erased most of the type parameters, but by exposing all those types now, it forces us to use dynamic dispatch which limits a lot of use cases and puts owness on the callers. The goal was eliminate locking but given the network IO, I don't think it's worth the regression to the DX.

@heaths
Copy link
Member Author

heaths commented Nov 26, 2025

Actually, if the type params are the source of "ugliness" and complexity, then the goal is to eliminate them. If I use fewer in the declaration, I think can limit it to P: Page and C: AsRef<str> - the latter because most implements do/will use Url for a "next link" instead of some arbitrary String for an opaque continuation token, but we know the latter exists in Azure.

So, it might mean that Pager gains an extra type param - that part of the implementation leaks out into the API - but we can at least default that as well e.g.,

pub type Pager<P, F = JsonFormat, C = Url> = ItemIterator<Response<P, F>, C>;

Cosmos uses a String but also defines their own type alias anyway, so they could do something like:

pub type FeedPager<P, F = JsonFormat, C = String> = ItemIterator<Response<P, F>, C>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Core The azure_core crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eliminating locking and improve safety with Pagers, Pollers

2 participants