Skip to content
Snippets Groups Projects

Modify igwn alert listener error handling to allow graceful shutdown

Closed Cody Messick requested to merge cody.messick/gwcelery:igwn_alert_graceful_shutdown into main

I think this MR fixes #554. I think two things were going wrong: One, I think switching from using a context manager (with blah.open() as s) to opening and closing the stream_obj had a little thinko, specifically I think the stream_obj just needed to be closed in the receiver's stop method instead of going in and trying to manually stop that object's private consumer object. Secondly, I think when the stream object is closed in the stop method, the try except clause in the while self.running loop needs to catch it the error and just pass to let the while loop exit (similar to what we found in !1278 (merged))

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Cody Messick added 2 commits

    added 2 commits

    • dfcc7f88 - 1 commit from branch emfollow:main
    • e39a1ebc - Modify igwn alert listener error handling to allow graceful shutdown. Closes #554

    Compare with previous version

  • We used to have something like what you have in this patch (see !905 (diffs)) and we moved away from it. I don't have the subtleties fresh in memory, but there were a few. At first look it seems like we are going back to what we had and hence I am a little hesitant to make changes to the first point you mention.

    Regarding the stop method, it was put in adc streaming consumer in this patch specifically to terminate when running in a different thread. If the same is called by the higher level hop consumer then we could use that (it did not back at the time).

    You second point is fine, if there is an error while closing the consumer we can pass that.

  • Is there any update for this? Is there there more work to be done here or should we close this? I think this problem has occurred less often since but still occasionally pops up.

  • Closed because we are no longer using a custom igwn alert client after !1407 (merged).

  • closed

Please register or sign in to reply
Loading