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

Add cancelation token overloads to appropriate async methods #3911

Open
rockfordlhotka opened this issue May 3, 2024 · 3 comments · May be fixed by #3926
Open

Add cancelation token overloads to appropriate async methods #3911

rockfordlhotka opened this issue May 3, 2024 · 3 comments · May be fixed by #3926
Assignees

Comments

@rockfordlhotka
Copy link
Member

There are a number of async methods where having an overload with a cancelation token may make sense.

This is only appropriate if the cancelation token can flow down to a system API that can actually cancel the operation.

It is generally not appropriate for the data portal, because it is not possible to flow the cancelation token across network boundaries. For example, indicating that someone could "cancel" a client-side data portal operation makes little sense, because maybe we could cancel the http or gprc or rabbitmq request on the client, but there's no way to actually cancel the operation on the server.

@StefanOssendorf
Copy link
Contributor

We should evaluate whether we can pass it along for a data portal operation. Yes, we can't transfer the token itself over the wire. But asp.net core for example provides a token which indicates whether the request has been closed/cancelled or not by the client. That token could be propagated on the server side. But I think that should be a different issue with more planning.

@luizfbicalho
Copy link
Contributor

I agree that this need more planning, my idea in this issue is just to add a cancellation token on every interface and implementation that uses a timeout, just like I did on the #3800
A second part would be to make the timeout version obsolete and start to propagate the cancellation token around.
The third part should be to remove the timeouts

I would just do the first part now

@StefanOssendorf
Copy link
Contributor

Yes, I understand that. I only wanted to raise awareness we could implement a proper cancellation mechanism througout the project. But we should move that into another issue.

Back to the part of this issue:
One method would be

public static async Task WaitForIdle(INotifyBusy source, TimeSpan timeout, [CallerMemberName] string methodName = "")

to be part of this enhancement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

3 participants