refactor(iroh): use boxed client to get rid of the C type parameter #2353
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # iroh/src/client.rs # iroh/src/client/authors.rs # iroh/src/client/blobs.rs # iroh/src/client/docs.rs # iroh/src/client/tags.rs # iroh/src/node.rs # iroh/src/node/builder.rs
rklaehn
force-pushed
the
boxed-client
branch
2 times, most recently
from
d4ecad6
to
4bada75
Compare
rklaehn
force-pushed
the
boxed-client
branch
2 times, most recently
from
12cf5de
to
de11d25
Compare
They are no longer needed and will be removed soon.
Frando
reviewed
Jun 20, 2024
document boxing overhead, some renaming, consistent use of type alias
|
@rklaehn please add some more details on the breaking changes, otherwise I have to go through the whole PR when writing the changelog |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Added some more text. Is that sufficient? |
|
thanks, looks great |
ppodolsky
pushed a commit
to izihawa/iroh
that referenced
this pull request
Jun 22, 2024
…0-computer#2353) ## Description I implemented a boxed connection and a boxed service endpoint in quic-rpc. With this we can get rid of the `<C: ServiceConnection<RpcService>` type parameter and make the quinn and mem client/server side the same type. The nice thing about this approach is that it will not lead to additonal boxing on the mem path, and for the quinn or whatever io path the boxing will probably not matter that much compared to all the other things going on. ## Breaking Changes Breaking changes affect solely the iroh client. The iroh client as well as all the subsystem clients no longer need a type parameter C to distinguish between an in memory connection and a remote connection. - Code that directly uses the clients should be unaffected. E.g. `iroh.blobs().read(hash)` will still compile. - Code that takes a client as an argument will have to be modified to remove the type parameter. E.g. ```rust fn download<C>(client: blobs::Client<C>) where C: ... ``` will become just ```rust fn download(client: blobs::Client) ``` The type aliases `iroh::client::MemIroh` and `iroh::client::QuicIroh` for an iroh client specialized for memory or remote use have been retained, but will be removed in one of the next releases. In detail: - iroh::client::Iroh loses the `C` type parameter - iroh::client::blobs::Client loses the `C` type parameter - iroh::client::tags::Client loses the `C` type parameter - iroh::client::authors::Client loses the `C` type parameter - iroh::client::docs::Client loses the `C` type parameter ## Notes & open questions Note: I marked the old type aliases MemIroh, QuicIroh etc as deprecated. That does not seem to actually do anything, but just serves as a reminder to remove them in the near future. ## Change checklist - [x] Self-review. - [x] Documentation updates if relevant. ~~- [x] Tests if relevant.~~ - [x] All breaking changes documented.
matheus23
pushed a commit
that referenced
this pull request
Nov 14, 2024
…2353) ## Description I implemented a boxed connection and a boxed service endpoint in quic-rpc. With this we can get rid of the `<C: ServiceConnection<RpcService>` type parameter and make the quinn and mem client/server side the same type. The nice thing about this approach is that it will not lead to additonal boxing on the mem path, and for the quinn or whatever io path the boxing will probably not matter that much compared to all the other things going on. ## Breaking Changes Breaking changes affect solely the iroh client. The iroh client as well as all the subsystem clients no longer need a type parameter C to distinguish between an in memory connection and a remote connection. - Code that directly uses the clients should be unaffected. E.g. `iroh.blobs().read(hash)` will still compile. - Code that takes a client as an argument will have to be modified to remove the type parameter. E.g. ```rust fn download<C>(client: blobs::Client<C>) where C: ... ``` will become just ```rust fn download(client: blobs::Client) ``` The type aliases `iroh::client::MemIroh` and `iroh::client::QuicIroh` for an iroh client specialized for memory or remote use have been retained, but will be removed in one of the next releases. In detail: - iroh::client::Iroh loses the `C` type parameter - iroh::client::blobs::Client loses the `C` type parameter - iroh::client::tags::Client loses the `C` type parameter - iroh::client::authors::Client loses the `C` type parameter - iroh::client::docs::Client loses the `C` type parameter ## Notes & open questions Note: I marked the old type aliases MemIroh, QuicIroh etc as deprecated. That does not seem to actually do anything, but just serves as a reminder to remove them in the near future. ## Change checklist - [x] Self-review. - [x] Documentation updates if relevant. ~~- [x] Tests if relevant.~~ - [x] All breaking changes documented.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
I implemented a boxed connection and a boxed service endpoint in quic-rpc.
With this we can get rid of the
<C: ServiceConnection<RpcService>type parameter and make the quinn and mem client/server side the same type.The nice thing about this approach is that it will not lead to additonal boxing on the mem path, and for the quinn or whatever io path the boxing will probably not matter that much compared to all the other things going on.
Breaking Changes
Breaking changes affect solely the iroh client.
The iroh client as well as all the subsystem clients no longer need a type parameter C to distinguish between an in memory connection and a remote connection.
Code that directly uses the clients should be unaffected. E.g.
iroh.blobs().read(hash)will still compile.Code that takes a client as an argument will have to be modified to remove the type parameter. E.g.
will become just
The type aliases
iroh::client::MemIrohandiroh::client::QuicIrohfor an iroh client specialized for memory or remote use have been retained, but will be removed in one of the next releases.In detail:
Ctype parameterCtype parameterCtype parameterCtype parameterCtype parameterNotes & open questions
Note: I marked the old type aliases MemIroh, QuicIroh etc as deprecated. That does not seem to actually do anything, but just serves as a reminder to remove them in the near future.
Change checklist
- [x] Tests if relevant.