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

Manage connection string using type converter #3

Merged
merged 6 commits into from
Dec 19, 2022

Conversation

glopesdev
Copy link
Member

@glopesdev glopesdev commented Dec 15, 2022

This PR refactors the management of connection strings in the editor using a type converter instead of a custom type. All functionality should be preserved, except that all connection string property types are now string.

This has the advantage of making operator properties more compatible when computing connection strings dynamically (e.g. loading from a file, etc).

The sub-properties Host and Port were also fused into a more general Address property, since there is a wider family of weirdly formatted connection strings which are possible in ZMQ that do not conform to the basic pattern hostname:port (see example section in zmq_pgm docs).

@glopesdev glopesdev added the feature New planned feature label Dec 15, 2022
@glopesdev glopesdev added this to the 0.1.0 milestone Dec 15, 2022
@glopesdev glopesdev requested a review from RoboDoig December 15, 2022 20:44
@glopesdev
Copy link
Member Author

glopesdev commented Dec 18, 2022

Two other points for consideration:

  1. Most socket types provide a default action paired with their natural complementary socket (e.g. Bind for Publisher and Connect for Subscriber). This means most of the time connection strings can ignore the bind/connect character and simply use the raw protocol://address string.
  2. The connection string can actually be used to specify multiple endpoints, with each endpoint separated by a comma. This is arguably more rare, but should probably be considered when using the type converter, so it does not parse away the second connection by mistake.

Given 1. and 2. it might also not be unreasonable to just provide the default connection string with no type converter and explain the syntax of connection strings in documentation.

@glopesdev glopesdev changed the title Refactor connection string management through a type converter Manage connection string using type converter Dec 19, 2022
@glopesdev glopesdev merged commit 2b8105e into bonsai-rx:main Dec 19, 2022
@glopesdev glopesdev deleted the connection-strings branch December 19, 2022 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New planned feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants