-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
autorelay: refactor libp2p.EnableAutoRelay
option
#1866
Comments
libp2p.EnableAutoRelay
optionlibp2p.EnableAutoRelay
option
Suggestion: I'd be grateful for better naming suggestions :) |
Preference for |
@sukunrt Flagging this for you in case you are interested in taking on more issues. Thanks for all the great contributions up till now, no pressure to take this up however 🙏 |
sure! I'd love to! |
As I understand. We want two new functions to configure autorelay
We would need to add the second options parameter because otherwise there would be no way of providing options like WithCircuitV1Support() In autorelay.WithPeerSource there is a configurable mininterval for callbacks which is set to 30 seconds in autorelay.WithStaticRelays. Should I allow for setting this minInterval too and make the function:
What should I do with the old function libp2p.EnableAutoRelay? Deprecate it or remove it? |
That's correct.
I'd prefer to have this as a
If it's not too much of a hassle to keep it around (as a deprecated function), I'd say let's just deprecate it. That will make the upgrade process smoother for our users. |
Can I change autorelay.WithPeerSource from
to
and set a default minInterval with autorelay.defaultConfig |
We should split the function into two options, one that requires a list of static relays and one that requires a peer source. Having options, some of which are required, is not a nice API pattern.
Originally posted by @marten-seemann in #1852 (comment)
The text was updated successfully, but these errors were encountered: