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

Drop broker::peer_flags? #314

Open
Neverlord opened this issue Feb 27, 2023 · 1 comment
Open

Drop broker::peer_flags? #314

Neverlord opened this issue Feb 27, 2023 · 1 comment

Comments

@Neverlord
Copy link
Member

While working on #309, I've noticed this little enum lying around in the code base:

enum class peer_flags : int {
  invalid = 0x00,
  local = 0x01,
  remote = 0x02,
  outbound = 0x04,
  inbound = 0x08,
};

It seems like nothing ever uses this. endpoint::peers() returns a list of peer_info objects, that technically contains this information:

/// Information about a peer of an endpoint.
/// @relates endpoint
struct peer_info {
  endpoint_info peer; ///< Information about the peer.
  peer_flags flags;   ///< Details about peering relationship.
  peer_status status; ///< The current peering status.
};

However, I found nothing that ever reads the flags member with the only exception being the Python bindings for exposing them to Python users.

Moreover, the enum values strike me as a leftover of previous Broker design iterations. There's no such thing as "local", "inbound" or "outbound" peerings. The only distinction we could make at the moment is for native vs WebSocket peers. Even for those, I wouldn't re-purpose the enum type, though. If we were to extend peer_info, I think we should go beyond a simple enum. If we wanted to expose native/WebSocket distinction, I think we should also pass additional information like client URI/version/etc.

Do I miss something or can we get rid of this as well when removing the Python module?

@Neverlord
Copy link
Member Author

I'll postpone this until we get rid of the Python bindings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant