Should Pager take a lifetime or owned ClientMethodOptions

[heaths]

Before GA we need to decide if Pager should take a lifetime parameter in order to reference the original ClientMethodOptions (rather, wrapper options) that was passed to the client method that created the Pager, or keep the existing into_owned(self) -> Self<'static> method defined in the emitter today.

See https://github.com/Azure/typespec-rust/pull/147/files#r1835097479 for context.

[heaths]

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=8947125dcdfa4ad98bf208f8c2918bc2 might be another way to do this efficiently: a generic helper to deserialize just the next link.

[analogrelay]

Another factor to consider here is that as long as Pager requires 'static lifetime, you have to be able to transfer ownership of everything you use in the make_request callback to the Pager. That means you have to clone everything you use from your Client's own fields. The pipeline, any client options, any other data. If that data is large, this may mean unexpected or even undesirable cloning.

I'm not saying that's a dealbreaker, but it's another reason to consider a lifetime argument. Using a lifetime argument would tie the Pager's lifetime to that of the Client that generated it, which does seem reasonable to me.

If we want a kind of "best of both worlds" approach, we could give Pager a lifetime argument and require that the Fn and Future be Clone. This would require that everything they capture be Clone (which would be enforced by the compiler). It would mean Pager<'a> is still coupled to the Client's lifetime, but it would allow us to add an .into_owned() method to Pager<'a> which converts it to Pager<'static> at the cost of some cloning.

[heaths]

It would mean Pager<'a> is still coupled to the Client's lifetime, but it would allow us to add an .into_owned() method to Pager<'a> which converts it to Pager<'static> at the cost of some cloning.

I think that's a reasonable expectation. I mean, it's not strictly required conceptually - you could create a client just to get a pager - but that would be atypical given typical usage patterns I've seen.

That said, there's really not much to clone anyway: client options are used to construct a Pipeline which is basically just a Vec<Arc<dyn Policy> anyway, so fairly cheap to clone. The endpoint is just a url::Url which is also pretty light. I wouldn't expect most cases to have much more data to ref-count or copy (clone).

[analogrelay]

That said, there's really not much to clone anyway

So far, I mostly agree. That's all I've had to clone to make Pagers, the pipeline and the "base" Request (which is just the URL plus headers). Cosmos may get a little heavier than that, since I haven't implemented the failover and region-selection logic we have in other clients. That logic requires some caches (the endpoints available in each region) and other more complex policy. Still, even looking at the Go SDK version of the globalEndpointManager struct, it's likely not a lot to clone, and could just be put behind an Arc to make clones lightweight.

[heaths]

Don't get me wrong: I think lifetime parameters are fine. I've only pressed lightly (IMO) to avoid them to make the API easier but, where they are helpful and make a significant impact - by some definition of "significant" - we absolutely should use them.