swarm/handler: Rename `InEvent` and `OutEvent` to `FromBehaviour` and `ToBehaviour` #2854

thomaseizinger · 2022-08-30T09:20:40Z

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.

The text was updated successfully, but these errors were encountered:

mxinden · 2022-09-02T03:50:34Z

I find this more intuitive looking at ConnectionHandler only. Though in the greater picture, including NetworkBehaviour, it is inconsistent with NetworkBehaviour::OutEvent.

thomaseizinger · 2022-09-02T06:31:49Z

I find this more intuitive looking at ConnectionHandler only. Though in the greater picture, including NetworkBehaviour, it is inconsistent with NetworkBehaviour::OutEvent.

We can name that one ToSwarm? :)

elenaf9 · 2022-09-02T14:51:10Z

+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.

elenaf9 · 2022-09-02T15:45:35Z

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.

thomaseizinger · 2022-09-04T09:36:47Z

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.

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.

mxinden · 2022-09-05T06:10:57Z

Related to this: What do you think about renaming NetworkBehaviour::inject_event -> ::inject_handler_msg

With #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 #2832 (comment).

mxinden · 2022-09-05T06:11:42Z

I am in favor of the proposals in this issue.

thomaseizinger · 2022-09-05T17:41:34Z

Related to this: What do you think about renaming NetworkBehaviour::inject_event -> ::inject_handler_msg

With #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?

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.

elenaf9 · 2022-09-07T08:42:52Z

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)

I'd be in favor of this.

jxs · 2022-11-04T14:05:03Z

Related to this: What do you think about renaming NetworkBehaviour::inject_event -> ::inject_handler_msg

With #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?

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.

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

mxinden · 2022-11-04T20:49:09Z

Good point @jxs. Doing the rename with the hard deprecation (i.e. removal) of the inject_ methods sounds good to me.

thomaseizinger · 2022-11-04T21:27:52Z

Sounds reasonable to me. +1

Mubelotix · 2022-11-07T17:20:28Z

I'm confused about why we keep saying Swarm, I thought the term was replaced by Switch

thomaseizinger · 2023-03-22T14:55:55Z

Planning this for the next milestone.

Previously, the associated types on `NetworkBehaviour` and `ConnectionHandler` carried generic names like `InEvent` and `OutEvent`. These names are _correct_ in that `OutEvent`s are passed out and `InEvent`s are passed in but they don't help users understand how these types are used. In theory, a `ConnectionHandler` could be used separately from `NetworkBehaviour`s but that is highly unlikely. Thus, we rename these associated types to indicate, where the message is going to be sent to: - `NetworkBehaviour::OutEvent` is renamed to `ToSwarm`: It describes the message(s) a `NetworkBehaviour` can emit to the `Swarm`. The user is going to receive those in `SwarmEvent::Behaviour`. - `ConnectionHandler::InEvent` is renamed to `FromBehaviour`: It describes the message(s) a `ConnectionHandler` can receive from its behaviour via `ConnectionHandler::on_swarm_event`. The `NetworkBehaviour` can send it via the `ToSwarm::NotifyHandler` command. - `ConnectionHandler::OutEvent` is renamed to `ToBehaviour`: It describes the message(s) a `ConnectionHandler` can send back to the behaviour via the now also renamed `ConnectionHandlerEvent::NotifyBehaviour` (previously `ConnectionHandlerEvent::Custom`) Resolves: #2854. Pull-Request: #3848.

Previously, the associated types on `NetworkBehaviour` and `ConnectionHandler` carried generic names like `InEvent` and `OutEvent`. These names are _correct_ in that `OutEvent`s are passed out and `InEvent`s are passed in but they don't help users understand how these types are used. In theory, a `ConnectionHandler` could be used separately from `NetworkBehaviour`s but that is highly unlikely. Thus, we rename these associated types to indicate, where the message is going to be sent to: - `NetworkBehaviour::OutEvent` is renamed to `ToSwarm`: It describes the message(s) a `NetworkBehaviour` can emit to the `Swarm`. The user is going to receive those in `SwarmEvent::Behaviour`. - `ConnectionHandler::InEvent` is renamed to `FromBehaviour`: It describes the message(s) a `ConnectionHandler` can receive from its behaviour via `ConnectionHandler::on_swarm_event`. The `NetworkBehaviour` can send it via the `ToSwarm::NotifyHandler` command. - `ConnectionHandler::OutEvent` is renamed to `ToBehaviour`: It describes the message(s) a `ConnectionHandler` can send back to the behaviour via the now also renamed `ConnectionHandlerEvent::NotifyBehaviour` (previously `ConnectionHandlerEvent::Custom`) Resolves: libp2p#2854. Pull-Request: libp2p#3848.

thomaseizinger changed the title ~~swarm/handler: Rename InEvent and OutEvent to FromBehaviour and `ToBehaviour~~ swarm/handler: Rename InEvent and OutEvent to FromBehaviour and ToBehaviour Aug 30, 2022

elenaf9 mentioned this issue Sep 2, 2022

swarm: Add FromFn ConnectionHandler #2852

Closed

10 tasks

mxinden mentioned this issue Sep 5, 2022

*: Prepare v0.48.0 #2869

Merged

5 tasks

elenaf9 mentioned this issue Sep 7, 2022

swarm/behaviour: Replace inject_* with on_event #2867

Closed

4 tasks

thomaseizinger mentioned this issue Sep 27, 2022

protocols/stream: Add a streaming-response protocol to rust-libp2p #2657

Closed

jxs mentioned this issue Nov 4, 2022

swarm/handler: Inject events via single method #3080

Closed

jxs mentioned this issue Nov 6, 2022

swarm/behaviour: Replace inject_* with on_event #3011

Merged

4 tasks

thomaseizinger mentioned this issue Nov 15, 2022

swarm: Rename NetworkBehaviourAction to ToSwarm #3123

Closed

mxinden added priority:important The changes needed are critical for libp2p, or are blocking another project difficulty:moderate help wanted getting-started Issues that can be tackled if you don't know the internals of libp2p very well labels Mar 13, 2023

thomaseizinger added this to the v0.52.0 milestone Mar 22, 2023

tcoratger mentioned this issue Apr 28, 2023

feat(swarm): rename associated types for message passing #3848

Merged

3 tasks

mergify bot closed this as completed in #3848 May 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

swarm/handler: Rename `InEvent` and `OutEvent` to `FromBehaviour` and `ToBehaviour` #2854

swarm/handler: Rename `InEvent` and `OutEvent` to `FromBehaviour` and `ToBehaviour` #2854

thomaseizinger commented Aug 30, 2022

mxinden commented Sep 2, 2022

thomaseizinger commented Sep 2, 2022

elenaf9 commented Sep 2, 2022 •

edited

Loading

elenaf9 commented Sep 2, 2022 •

edited

Loading

thomaseizinger commented Sep 4, 2022

mxinden commented Sep 5, 2022

mxinden commented Sep 5, 2022

thomaseizinger commented Sep 5, 2022 •

edited

Loading

elenaf9 commented Sep 7, 2022

jxs commented Nov 4, 2022

mxinden commented Nov 4, 2022

thomaseizinger commented Nov 4, 2022

Mubelotix commented Nov 7, 2022 •

edited

Loading

thomaseizinger commented Mar 22, 2023

swarm/handler: Rename InEvent and OutEvent to FromBehaviour and ToBehaviour #2854

swarm/handler: Rename InEvent and OutEvent to FromBehaviour and ToBehaviour #2854

Comments

thomaseizinger commented Aug 30, 2022

Description

Motivation

Current Implementation

Are you planning to do it yourself in a pull request?

mxinden commented Sep 2, 2022

thomaseizinger commented Sep 2, 2022

elenaf9 commented Sep 2, 2022 • edited Loading

elenaf9 commented Sep 2, 2022 • edited Loading

thomaseizinger commented Sep 4, 2022

mxinden commented Sep 5, 2022

mxinden commented Sep 5, 2022

thomaseizinger commented Sep 5, 2022 • edited Loading

elenaf9 commented Sep 7, 2022

jxs commented Nov 4, 2022

mxinden commented Nov 4, 2022

thomaseizinger commented Nov 4, 2022

Mubelotix commented Nov 7, 2022 • edited Loading

thomaseizinger commented Mar 22, 2023

swarm/handler: Rename `InEvent` and `OutEvent` to `FromBehaviour` and `ToBehaviour` #2854

swarm/handler: Rename `InEvent` and `OutEvent` to `FromBehaviour` and `ToBehaviour` #2854

elenaf9 commented Sep 2, 2022 •

edited

Loading

elenaf9 commented Sep 2, 2022 •

edited

Loading

thomaseizinger commented Sep 5, 2022 •

edited

Loading

Mubelotix commented Nov 7, 2022 •

edited

Loading