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

The default endpoints to listen on #1572

Closed
YuanYuYuan opened this issue Oct 30, 2024 · 4 comments · Fixed by #1623
Closed

The default endpoints to listen on #1572

YuanYuYuan opened this issue Oct 30, 2024 · 4 comments · Fixed by #1623
Labels
internal Changes not included in the changelog

Comments

@YuanYuYuan
Copy link
Contributor

YuanYuYuan commented Oct 30, 2024

Describe the release item

After migrating to 1.0, we force a zenoh session to listen on

{
  /// Which endpoints to listen on. E.g. tcp/0.0.0.0:7447.
  /// By configuring the endpoints, it is possible to tell zenoh which are the endpoints that other routers,
  /// peers, or client can use to establish a zenoh session.
  /// For TCP/UDP on Linux, it is possible additionally specify the interface to be listened to:
  /// E.g. tcp/0.0.0.0:7447#iface=eth0, for listen connection only on eth0
  listen: {
    /// The list of endpoints to listen on.
    /// Accepts a single list (e.g. endpoints: ["tcp/[::]:7447", "udp/[::]:7447"])
    /// or different lists for router, peer and client (e.g. endpoints: { router: ["tcp/[::]:7447"], peer: ["tcp/[::]:0"] }).
    ///
    /// See https://docs.rs/zenoh/latest/zenoh/config/struct.EndPoint.html
    endpoints: { router: ["tcp/[::]:7447"], peer: ["tcp/[::]:0"] },
}
  1. Users might not want to use tcp and build zenoh without the feature flag. Now they need to overwrite the default endpoints to address the "unsupported protocol" but it's not allowed to set an empty endpoint in the field. This means we force a peer-mode session always explicitly listen on specific endpoint.
  2. Following 1., this necessary change is not documented in the migration guide.
  3. Does exposing this detail really matter for user experience? We used to have a zenoh session in peer mode and don't need to specify its listening endpoints. It can either connect to the other sessions or listen on some endpoints if needed. Now we always need to set one if we don't use tcp in peer mode or we need to change the mode of session.
@YuanYuYuan YuanYuYuan added the release Part of the next release label Oct 30, 2024
@YuanYuYuan YuanYuYuan added internal Changes not included in the changelog and removed release Part of the next release labels Oct 30, 2024
@YuanYuYuan
Copy link
Contributor Author

@OlivierHecart what do you think?

@OlivierHecart
Copy link
Contributor

It is allowed to set an empty listen/endpoints array!
We changed this as before it was not possible to make a peer not listening to any endpoint (as setting an empty list was making it listen on the default). You can now explicitly set an empty array to make it not listen to anything.

Now it indeed force users to change the default when deactivating TCP feature. It is in my opinion not a big problem as TCP is kind of our default transport.
We could eventually make so that when TCP is disabled the configured TCP listener endpoints are ignored. But I'm not for it.

@YuanYuYuan
Copy link
Contributor Author

YuanYuYuan commented Nov 21, 2024

Follow-up

  1. @OlivierHecart's suggestion is correct that we can wipe out the default TCP listening endpoint by setting
{
listen: {
    endpoints: [""],
  }
}
  1. This pattern could be improved since our default config doesn't follow the compiling features. We will adjust the default config and warn the users that the TCP listening endpoints are neither used nor set according to the given config.

@OlivierHecart
Copy link
Contributor

It has been decided to change the configuration default values when building with feature transport_tcp disabled. See attached PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Changes not included in the changelog
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants