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

feat(request-response): Add support for custom dial options when making request to disconnected peer #5692

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nazar-pc
Copy link
Contributor

Description

We discussed #5634 at one of the community calls and the ways to approach it, one of the challenges we discussed was around the argument type and how to handle the case where PeerId is not specified in DialOpts.

I went with a relatively straightforward route: use PeerId when it is specified (same behavior as before this PR) and use ConnectionId otherwise, wrapping both into an enum to use as a key in internal HashMap.

Implementation ended up being fairly simple with the only breaking change being peer field in request_response::Event::OutboundFailure changing from PeerId to Option<PeerId>.

Resolves #5634

Notes & open questions

One thing I do not like too much and only did for backwards compatibility purposes is additional impl From<&PeerId> for DialOpts such that existing calls continue to work. If changing &peer_id to peer_id is an acceptable breaking change then that impl can be removed.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@nazar-pc nazar-pc force-pushed the request-response-with-addresses branch from d8d075f to 05fe824 Compare November 26, 2024 17:33
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

Successfully merging this pull request may close these issues.

Support explicit addresses for dialing purposes send_request
1 participant