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

Use topic terminology from ZeroMQ #605

Merged
merged 2 commits into from
Sep 18, 2018

Conversation

jamessynge
Copy link
Contributor

We were using both channel and msg_type to refer to what ZeroMQ
calls topics. No need for three terms, so this changes them
all to topic.

Fixes #597.

We were using both channel and msg_type to refer to what ZeroMQ
calls topics. No need for three terms, so this changes them
all to topic.
@jamessynge jamessynge requested a review from wtgee September 18, 2018 00:34
Copy link
Member

@wtgee wtgee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for getting this changed!

Messages are sent to channels, a name that can be used to allow
a high-level partitioning of messages. A channel name may not
include whitespace. Among the currently used channel names are:
Messages are sent to topics, a name that can be used to allow
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the best place to record all the current topics?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan is to develop a scheme for future topics and to also write general documentation about topics and filtering. I've created issue #606 for that.


* TEST-CHANNEL (test_messaging.py)
* Test-Topic (test_messaging.py)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we now have a mix of naming conventions. I know they are all allowed but should we have some logical scheme?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That change was deliberate, just to test that the title-case form was supported.

# Channel names must consist of the characters.
name_re = re.compile('[a-zA-Z][-a-zA-Z0-9_.:]*')
# Topic names must consist of the characters.
topic_name_re = re.compile('[a-zA-Z][-a-zA-Z0-9_.:]*')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need the r' with re.compile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raw strings are only needed when the string includes characters to be escaped AND you don't want to double escape them... or multi-line strings.

""" Create a listener

Args:
port (int): The port (on localhost) to bind to.
channel (str): Which topic channel to subscribe to.
topic (str): Which topic topic to subscribe to.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repeat of topic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@jamessynge jamessynge merged commit c1535f9 into panoptes:develop Sep 18, 2018
@jamessynge jamessynge deleted the issue-597-topic branch September 18, 2018 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants