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

Should bus.publish() raise BusShutdownException? #110

Open
m3d opened this issue Nov 17, 2018 · 3 comments
Open

Should bus.publish() raise BusShutdownException? #110

m3d opened this issue Nov 17, 2018 · 3 comments
Assignees
Labels

Comments

@m3d
Copy link
Member

m3d commented Nov 17, 2018

I was suprised that following code was never ending:

    def run(self):
        try:
            while True:
                self.bus.publish('tick', None)
                self.bus.sleep(self.sleep_time)
        except BusShutdownException:
            pass

Then I realized that there should be self.bus.is_alive() instead but ... do you remember why? Now I would propose to have both ... as far as I remember the is_alive() is for "boundary" nodes which have only external inputs i.e. they do not use listen() and the external inputs could be very rare ... ???

@zbynekwinkler
Copy link
Member

I don't remember. Raising BusShutdownException even from publish seems like a good idea.

@m3d
Copy link
Member Author

m3d commented Nov 20, 2018

Well, thanks to failing test #112 I now see the difference between listen and publish. For listen the module/node did not stop immediately - it waited until the queue was processed to the termination None message.

@zbynekwinkler
Copy link
Member

Hmm, hmm. Interesting. So when a shutdown is requested, node is allowed to call publish but if all nodes were requested to shutdown, the published message is not going to be read. But it is going to be in the log file. While the None used to denote shutdown is not going to be in the log file. It is somewhat interesting we are still able to replay all the way to the end.

I propose to refactor/redesign/fix this during the refactoring we have been discussing lately where nodes would be required to inherit from a common base class.

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

No branches or pull requests

2 participants