refactor(request-response): don't use upgrade infrastructure#3914
refactor(request-response): don't use upgrade infrastructure#3914mergify[bot] merged 34 commits intomasterfrom
Conversation
mxinden
left a comment
mxinden
left a comment
There was a problem hiding this comment.
Direction looks good to me! Assuming that you don't want a final review yet.
A high-level review would be appreciated :) |
|
@mxinden This is ready for review and is the 2nd last piece towards starting the work around a simpler design of |
mxinden
left a comment
mxinden
left a comment
There was a problem hiding this comment.
We need to bound the number of concurrent inbound streams. Otherwise this change looks good to me.
(Surprised that this ends up being 2 lines less instead of many new lines in addition.)
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏 |
|
This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏 |
…libp2p into refactor/req-res-on-upgrade
|
This needs some more thinking on how we want to report errors. Also, I noticed that several of the enum variants are actually unused so we should properly clean this up. I'll see to look into it next week. Help much appreciated though :) |
This comment was marked as resolved.
This comment was marked as resolved.
|
@thomaseizinger let me know once this is ready for another review. |
It is! :) |
mxinden
left a comment
mxinden
left a comment
There was a problem hiding this comment.
Great work! Happy for this to land.
This patch refactors `libp2p-request-response` to not use the "upgrade infrastructure" provided by `libp2p-swarm`. Instead, we directly convert the negotiated streams into futures that read and write the messages. Related: libp2p#3268. Related: libp2p#2863. Pull-Request: libp2p#3914. Co-authored-by: Yiannis Marangos <[email protected]>
4 participants
Description
This patch refactors
libp2p-request-responseto not use the "upgrade infrastructure" provided bylibp2p-swarm. Instead, we directly convert the negotiated streams into futures that read and write the messages.Related: #3268.
Related: #2863.
Attributions
Co-authored-by: Yiannis Marangos [email protected]
Notes & open questions
I am putting this up as an initial draft. I think some clean-up can be done but overall, I am glad it is working this well.Change checklist