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

refactor: remove redundant SigInt handler #89

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

kentbull
Copy link
Contributor

Since the Doist loop listens for the KeyboardInterrupt exception then it effectively handles the Ctrl-C SigInt interrupt already which means the shutdown handler only has to handle the SigTerm signal.

Since the Doist loop listens for the KeyboardInterrupt exception then it effectively handles the Ctrl-C SigInt interrupt already which means the shutdown handler only has to handle the SigTerm signal.
@kentbull kentbull merged commit a51a698 into WebOfTrust:main Jan 13, 2025
2 checks passed
@kentbull kentbull deleted the cleanup-sigint-handler branch January 13, 2025 03:56
@SmithSamuelM
Copy link
Collaborator

As I posted on the other related repo, The Doer sigterm handler should either raise KeyboardInterrupt or call sys.exit() to trigger doist.exist instead of calling doist.exit() directly.

@kentbull
Copy link
Contributor Author

Thanks for pointing that you. Yes, I remember now, you did say that. I'll post a PR later this week with my draft at having an agency properly close itself. Having the GracefulShutdownDoer raise a KeyboardInterrupt is the easy part. The hard part is in having each Agent and then the Agency shut themselves down in the proper order.

@SmithSamuelM
Copy link
Collaborator

SmithSamuelM commented Jan 13, 2025

The ordering is already taken care of by the Doist. The order of the enter calls is reversed for the exit calls. So if you enter doers in the order of dependency where if C is dependent on B and B is dependent on A then you want to enter them in the order A - B - C. Then when doist.exit gets called it will exit its doers in the reverse order. C - B - A. This avoids race conditions where C depends on a resource in order to exit cleanly but that resource has already exited.

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