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

consider removing built-in cancellation mechanism #118

Open
danburkert opened this issue Mar 6, 2023 · 4 comments
Open

consider removing built-in cancellation mechanism #118

danburkert opened this issue Mar 6, 2023 · 4 comments

Comments

@danburkert
Copy link
Contributor

Most of the client APIs appear to take an optional cancellation token. Tokio's CancellationToken type is designed to allow alerting a spawned task that cancellation is being requested. Because google-cloud-rust isn't spawning tasks, it's not necessary for a cancellation mechanism to be built in to the library. Instead, users of the library can simply drop the futures returned from the google-cloud-rust APIs, perhaps in response to a CancellationToken firing, to enable cancellation.

@yoshidan
Copy link
Owner

yoshidan commented Mar 7, 2023

Thank you for your suggestion, I was assuming a case like Go's context, where the parent's CancellationToken is passed to the child task when the task is generated internally.
However, in the case of a simple API call, the library does not generate the task internally, so I would like to modify it so that unnecessary CancellationToken is not passed to the child task.

@danburkert
Copy link
Contributor Author

I'm not a Go expert, but I believe their async system works differently, and does require the child task to have an out-of-band explicit cancellation mechanism. Rust futures can be cancelled simply by dropping them. This is why e.g. tonic and reqwest don't have cancellation mechanisms built in.

@fredr
Copy link
Contributor

fredr commented Mar 7, 2023

When I added the pub/sub MessageStream I hid the CancellationToken within the struct, and cancel it when it is dropped. Then the user of the API doesn't have to think about it.

See 423a559

I see now that I could have stored a DropGuard instead, then we wouldn't need to implement Drop

@danburkert
Copy link
Contributor Author

@fredr it's possible that cancellation doesn't need to be built in at all. If you have tasks communicating via channels, you can have task shutdown automatically when the remote end of the channel is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants