Conversation
Handle SIGTERM in startservers.py. Forcibly remove rsyslog pid to avoid error.
cpu
left a comment
cpu
left a comment
There was a problem hiding this comment.
There seems to be a strange CI error on the config-next build task:
ERROR: for bmysql Cannot create container for service bmysql: device or resource busy
| DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" | ||
|
|
||
| # start rsyslog | ||
| rm -f /var/run/rsyslogd.pid |
There was a problem hiding this comment.
Could you expand on the comment above the rm to explain briefly why we are trying to remove this pid file?
Given that we expect this rm to fail in most cases do you think it's also worth redirecting stderr to /dev/null so that nobody will confuse the rm: cannot remove '/var/run/rsyslog.pid': No such file or directory as an error to be investigated?
There was a problem hiding this comment.
Added the comment. rm -f will inhibit errors when the file is not present.
| startup. Anything that did start before this point can be cleaned | ||
| up explicitly by calling stop(), or automatically atexit. | ||
| """ | ||
| signal.signal(signal.SIGTERM, lambda _, __: stop()) |
There was a problem hiding this comment.
Could you also register this for signal.SIGINT so it is useful outside of docker as well.
- Handle SIGINT too. - Use unbuffered mode for Python so the print statements (like "all servers running") get printed right away rather than at shutdown - Squelch an unnecessary OSError about interrupting the wait() call.
|
I addressed @rolandshoemaker's comment and added some other substantive changes, please re-review. |
3 participants
Uh oh!
There was an error while loading. Please reload this page.