-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix shutdown #335
Fix shutdown #335
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main DiamondLightSource/blueapi#335 +/- ##
==========================================
- Coverage 86.61% 86.52% -0.09%
==========================================
Files 41 41
Lines 1554 1559 +5
==========================================
+ Hits 1346 1349 +3
- Misses 208 210 +2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Could do with a test or two
fc593cf
to
46b2e97
Compare
I've added in a small check using the is_connected method. However, I am going to change up the stomp connection part slightly in another PR so it doesn't crash if you try start BlueApi without an activemq instance running. The tests will be changed/added too acordingly there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and should be merged if it fixes a common failure case.
See minor nitpicks
Co-authored-by: Callum Forrester <callum.forrester@diamond.ac.uk>
Relates to #220 . This change is to enable blueapi to be shutdown with crtl+c when the activemq drops and it can no longer connect to it.