swarm/handler: Rename `InEvent` and `OutEvent` to `FromBehaviour` and `ToBehaviour`
Description
Rename the associated types of ConnectionHandler from InEvent to FromBehaviour and OutEvent to ToBehaviour.
Motivation
Using a ConnectionHandler outside of the NetworkBehaviour abstraction is generally possible put very unlikely. To make it easier for users to understand, what these types do, we can rename them to not just be an "in" and "out" but to include where the event is going to be handled or where it is coming from.
Current Implementation
Are you planning to do it yourself in a pull request?
Maybe.
I find this more intuitive looking at ConnectionHandler only. Though in the greater picture, including NetworkBehaviour, it is inconsistent with NetworkBehaviour::OutEvent.
I find this more intuitive looking at
ConnectionHandleronly. Though in the greater picture, includingNetworkBehaviour, it is inconsistent withNetworkBehaviour::OutEvent.
We can name that one ToSwarm? :)
+1 in favor of using more concrete names for associated types.
Maybe we could instead of FromBehaviour name InEvent -> ToHandler. If we then also follow @thomaseizinger proposal to rename NetworkBehaviour::OutEvent to ToSwarm we'd have consistend names ToHandler, ToBehaviour, ToSwarm, etc.
Similar naming is also used in the relay v2 protocol for events between handler and listeners.
Related to this: What do you think about renaming NetworkBehaviour::inject_event -> ::inject_handler_msg and ConnectionHandler::inject_event -> inject_behaviour_msg (or something similar).
When first working with NetworkBehaviour and Handler as a user I struggled quite a bit with understanding how behaviour and handler are related and exchange messages. As @thomaseizinger wrote above, there is no real use-case for a ConnectionHandler outside of a NetworkBehaviour, so I think we can be a bit less abstract with our names here.
When first working with
NetworkBehaviourandHandleras a user I struggled quite a bit with understanding how behaviour and handler are related and exchange messages.
Same! Especially the ConnectionHandlers OutEvent is obviously the NetworkBehaviour's InEvent. Although that is inferred through the associated type, it is confusing because the NetworkBehaviour has its own OutEvent.
Related to this: What do you think about renaming
NetworkBehaviour::inject_event->::inject_handler_msg
With https://github.com/libp2p/rust-libp2p/pull/2867/ NetworkBehaviour::inject_event would move to NetworkBehaviour::on_event via InEvent::ConnectionHandler. (Where InEvent would be renamed to FromSwarm with this proposal I am guessing.) Would you be in favor of that @elenaf9?
and
ConnectionHandler::inject_event->inject_behaviour_msg(or something similar).
I suggest we do the same change on ConnectionHandler, see also https://github.com/libp2p/rust-libp2p/issues/2832#issuecomment-1236252507.
I am in favor of the proposals in this issue.
Related to this: What do you think about renaming
NetworkBehaviour::inject_event->::inject_handler_msgWith #2867
NetworkBehaviour::inject_eventwould move toNetworkBehaviour::on_eventviaInEvent::ConnectionHandler. (WhereInEventwould be renamed toFromSwarmwith this proposal I am guessing.) Would you be in favor of that @elenaf9?
Yes, I think that makes sense. The only weird part is that FromSwarm will have a ConnectionHandler variant that will use the FromHandler event. Perhaps #2867 should introduce two event handlers?
-
NetworkBehaviour::on_swarm_event(FromSwarm) -
NetworkBehaviour::on_handler_event(FromHandler)
and
ConnectionHandler::inject_event->inject_behaviour_msg(or something similar).I suggest we do the same change on
ConnectionHandler, see also #2832 (comment).
I suggest we try and pack all of these into a single release.
Yes, I think that makes sense. The only weird part is that FromSwarm will have a ConnectionHandler variant that will use the FromHandler event. Perhaps https://github.com/libp2p/rust-libp2p/pull/2867 should introduce two event handlers?
NetworkBehaviour::on_swarm_event(FromSwarm)NetworkBehaviour::on_handler_event(FromHandler)
I'd be in favor of this.
Related to this: What do you think about renaming
NetworkBehaviour::inject_event->::inject_handler_msgWith #2867
NetworkBehaviour::inject_eventwould move toNetworkBehaviour::on_eventviaInEvent::ConnectionHandler. (WhereInEventwould be renamed toFromSwarmwith this proposal I am guessing.) Would you be in favor of that @elenaf9?Yes, I think that makes sense. The only weird part is that
FromSwarmwill have aConnectionHandlervariant that will use theFromHandlerevent. Perhaps #2867 should introduce two event handlers?
NetworkBehaviour::on_swarm_event(FromSwarm)NetworkBehaviour::on_handler_event(FromHandler)and
ConnectionHandler::inject_event->inject_behaviour_msg(or something similar).I suggest we do the same change on
ConnectionHandler, see also #2832 (comment).I suggest we try and pack all of these into a single release.
Being the rename of the associated types a breaking change by itself, should it be addressed when we soft (#3011/#3085) or hard deprecate the inject_* methods? If we pack it with #3011/#3085 then it's probably redundant to soft deprecate the inject_* methods since there will be a breaking change that implies touching the traits
Good point @jxs. Doing the rename with the hard deprecation (i.e. removal) of the inject_ methods sounds good to me.
Sounds reasonable to me. +1
I'm confused about why we keep saying Swarm, I thought the term was replaced by Switch
Planning this for the next milestone.