-
Notifications
You must be signed in to change notification settings - Fork 460
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
fix!: rename "transient" connections to "limited" #2645
Conversation
I've backed away from removing const stream = await libp2p.dialProcotol(peerId, '/my/protocol/1.0.0') In the above since you're dialing the protocol on the remote peer and not a multiaddr, you don't know ahead of time if you are connected or if the connection is relayed/direct/limited so the Happy to discuss further if anyone has a better idea for the API. |
To better align with [[email protected]](https://github.com/libp2p/go-libp2p/releases/tag/v0.34.0) rename "transient" connections to "limited". BREAKING CHANGE: There are three breaking API changes: * Connections have an optional `.limits` property * The `runOnTransientConnection` property of `libp2p.handle` and `libp2p.dialProtocol` has been renamed to `runOnLimitedConnection` * The `notifyOnTransient` property of `libp2p.register` has been renamed `notifyOnLimitedConnection` Refs: #2622
692bdb4
to
f24a06e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
-
A minor comment adjustment here
-
we don't really have any docs around this but I suppose we still should have a migration guide correct?
I think your rationale is sound. Though I'm not sure where the "removing" work was happening or what that consisted of. I'm guessing you were thinking of removing
Do we have a best practice or prior art for whether we default API decisions to a "try, analyze, retry" vs "config for goal success" ? I know we don't like to "config all the things" but it seems like js-libp2p likes to go the "config for goal success" route: i.e. when we make a function call, we want devs to pass whatever they need to ensure they can achieve their ideal outcome on first try. so.. for |
See the linked issue mentioned in the op.
Best practice is generally to avoid network operations wherever possible as I/O is the death of performance. In this case to make two attempts at opening a stream (and potentially a connection) could be very expensive so should be avoided. |
To better align with [[email protected]](https://github.com/libp2p/go-libp2p/releases/tag/v0.34.0) rename "transient" connections to "limited". BREAKING CHANGE: There are three breaking API changes: * Connections have an optional `.limits` property * The `runOnTransientConnection` property of `libp2p.handle` and `libp2p.dialProtocol` has been renamed to `runOnLimitedConnection` * The `notifyOnTransient` property of `libp2p.register` has been renamed `notifyOnLimitedConnection` Refs: #2622
To better align with [[email protected]](https://github.com/libp2p/go-libp2p/releases/tag/v0.34.0) rename "transient" connections to "limited". BREAKING CHANGE: There are three breaking API changes: * Connections have an optional `.limits` property * The `runOnTransientConnection` property of `libp2p.handle` and `libp2p.dialProtocol` has been renamed to `runOnLimitedConnection` * The `notifyOnTransient` property of `libp2p.register` has been renamed `notifyOnLimitedConnection` Refs: #2622
To better align with [[email protected]](https://github.com/libp2p/go-libp2p/releases/tag/v0.34.0) rename "transient" connections to "limited". BREAKING CHANGE: There are three breaking API changes: * Connections have an optional `.limits` property * The `runOnTransientConnection` property of `libp2p.handle` and `libp2p.dialProtocol` has been renamed to `runOnLimitedConnection` * The `notifyOnTransient` property of `libp2p.register` has been renamed `notifyOnLimitedConnection` Refs: #2622
To better align with [email protected] rename "transient" connections to "limited".
BREAKING CHANGE: There are three breaking API changes:
.transient
boolean property has been replaced with a.limits
object that may be undefined or may have.bytes
and/or.seconds
properties - the value of these properties decreases as the limits are used uprunOnTransientConnection
property oflibp2p.handle
andlibp2p.dialProtocol
has been renamed torunOnLimitedConnection
notifyOnTransient
property oflibp2p.register
has been renamednotifyOnLimitedConnection
Refs: #2622
Change checklist