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

Raise an exception if no sockets to poll #23

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

loerum
Copy link
Contributor

@loerum loerum commented Jul 18, 2019

Trying to receive messages from a not registered service will create an infinite loop, the following code will print None forever:

from posttroll.subscriber import Subscribe
with Subscribe("invalid_service", "counter",) as sub:
for msg in sub.recv():
    print(msg)

It's fixed by raising an exception in Subscriber.recv if no socket to poll.

I'm some what surprised that we first see this "feature" now ... so please check if I have missed something.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 79.62% when pulling d34f427 on fix-infinite-loop into 516725e on master.

@loerum
Copy link
Contributor Author

loerum commented Jul 30, 2019

By the way, the above infinite loop, would not have happen if an addr_listener was specified:

from posttroll.subscriber import Subscribe
with Subscribe("invalid_service", "counter", addr_listener=True) as sub:
for msg in sub.recv():
    print(msg)

In this case, there is always something to poll. Maybe if a name-server is been used, addr_listener should have the default value True ?

@mraspaud
Copy link
Member

I'm not in the office, please remind me to look at this next week

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

Thanks for this, could you add a small unit test for this @loerum ?

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

Successfully merging this pull request may close these issues.

3 participants