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

Azure core poller flux to support context passing #9762

Closed
samvaity opened this issue Apr 2, 2020 · 7 comments
Closed

Azure core poller flux to support context passing #9762

samvaity opened this issue Apr 2, 2020 · 7 comments
Assignees
Labels
APIChange This PR contains an addition or change to the API signature and must be reviewed by an architect. Azure.Core azure-core Client This issue points to a problem in the data-plane of the library. design-discussion An area of design currently under discussion and open to team and community feedback.

Comments

@samvaity
Copy link
Member

samvaity commented Apr 2, 2020

Update the current implementation of PollerFlux to support context passing for callback operations.

@alzimmermsft alzimmermsft added Azure.Core azure-core Client This issue points to a problem in the data-plane of the library. labels Apr 2, 2020
@triage-new-issues triage-new-issues bot removed the triage label Apr 2, 2020
@samvaity
Copy link
Member Author

samvaity commented May 5, 2020

@anuchandy Do you know if we would be able to get this in to start supporting context passing in LRO's for Form recognizer this beta?

@anuchandy
Copy link
Member

@samvaity like we discussed offline I'm starting on it today, will open a pr soon.

@anuchandy
Copy link
Member

anuchandy commented May 8, 2020

Sharing the update so far:

"Async" PollerFlux

PollerFlux is a Flux that users can subscribe with Reactor.Context. This reactor context is automatically available for activation/poll/getFinalResult functions, these functions could just use FluxUtils.withContext (to convert to Azure.Context) and make the raw service method call inside it. I think no issue exists in async space.

SyncPoller

In sync APIs we don't have a way to pass Azure.Context. We need to understand the granularity of control we need to provide via context. Depending on the scope of control, the context could be global to a single SyncPoller or can local to methods in SyncPoller or both. This design depends on clarity on use cases:

  1. The tracing for LRO.
    --> Tracking issue - [Design] Tracing story for LRO #10969. It is opened for PollerFlux but decisions should cover SyncPoller as well.
  2. The need for cancellation support for APIs exposed in SyncPoller.
    --> require understanding of [in-progress] sync cancellation feature.
  3. The telemetry of these APIs exposed in SyncPoller.

\\cc @JonathanGiles

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Jun 9, 2020

One complexity about PollerFlux is that after subscriberContext, it is no longer of class PollerFlux, but only a Flux.

This might be an issue, since caller now lose the flexibility of either use the async, or convert it to sync poller via getSyncPoller from PollerFlux.

I am pondering whether to add a Context at parameter to our builder of PollerFlux, so it would subscribe that Context to all its 4 operations (similar granularity question arises as for SyncPoller), while still returns the PollerFlux.

@alzimmermsft alzimmermsft added APIChange This PR contains an addition or change to the API signature and must be reviewed by an architect. design-discussion An area of design currently under discussion and open to team and community feedback. labels Jul 15, 2020
@joshfree
Copy link
Member

tagging @jianghaolu who is looking at polling

@weidongxu-microsoft
Copy link
Member

@alzimmermsft
Copy link
Member

Closing in favor of #10969

@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
APIChange This PR contains an addition or change to the API signature and must be reviewed by an architect. Azure.Core azure-core Client This issue points to a problem in the data-plane of the library. design-discussion An area of design currently under discussion and open to team and community feedback.
Projects
Status: Done
Development

No branches or pull requests

6 participants