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 support for setPipeline() #379

Closed
edevil opened this issue Mar 1, 2023 · 4 comments
Closed

Add support for setPipeline() #379

edevil opened this issue Mar 1, 2023 · 4 comments

Comments

@edevil
Copy link

edevil commented Mar 1, 2023

Context: capnproto/capnproto#1131

In the c++ capnproto lib this feature was added that allows, as @kentonv says:

This makes it possible, when an RPC forwards to another RPC, to start forward pipelined calls on the first to the second, before the first RPC actually completes.

Would it be possible to support this in capnproto-rust as well?

Thanks.

@kentonv
Copy link
Member

kentonv commented Mar 1, 2023

For a real-world example where this matters, compare request() and startRequest() in http-over-capnp.capnp.

https://github.com/capnproto/capnproto/blob/master/c%2B%2B/src/capnp/compat/http-over-capnp.capnp#L47

The introduction of setPipeline() made it possible to simplify this interface (request() being the new, simpler version, where startRequest() was what we needed before). However, it's currently not possible to implement request() in Rust.

@dwrensha
Copy link
Member

dwrensha commented Mar 2, 2023

Yeah, this should be possible. I took some some small first steps here: #380
Probably this will require a version bump, because we will need to change the signature of ClientHook.call() to additionally return a pipeline, and we will need to modify ResultsHook too.

@ErikMcClure
Copy link

We require this feature for our project. Unfortunately, #380 contains only stubs and minimal direction on what actually needs to happen to implement this feature. Would it be possible for @dwrensha to provide a more detailed explanation of everything that needs to change for this to be implemented correctly? I can then try my hand at providing a PR that implements this.

@dwrensha
Copy link
Member

dwrensha commented Sep 8, 2024

Implemented in #515, without modifying the return type of ClientHook.call().

@dwrensha dwrensha closed this as completed Sep 9, 2024
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

4 participants