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

Handle SIGTERM event and add test/oa in .dockerignore #46

Merged
merged 2 commits into from
Jun 11, 2019

Conversation

Joxit
Copy link
Member

@Joxit Joxit commented Jun 8, 2019

Only master can handle SIGTERM signals and I use process.exit because the close function is only returned by the listen function.

resolves #35

@missinglink
Copy link
Member

Nice!

Is it possible to avoid process.exit here?

@Joxit Joxit force-pushed the joxit/docker-sigterm branch from 15fc619 to 285cb29 Compare June 8, 2019 17:19
@Joxit
Copy link
Member Author

Joxit commented Jun 8, 2019

Yes it's possible but less understandable (I think). I added a new commit without process.exit.

It's a bit weird, when I use worker.kill('SIGTERM') we can't catch the SIGTERM event and the worker died instantaneously, so it equivalent to a process.exit.

I think this last commit is the best way to do a graceful shutdown.

@Joxit Joxit force-pushed the joxit/docker-sigterm branch from 285cb29 to 4a2f1b3 Compare June 9, 2019 13:49
Copy link
Member

@missinglink missinglink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested via docker-compose and works for me 👍

@missinglink missinglink merged commit 5db7432 into master Jun 11, 2019
@missinglink missinglink deleted the joxit/docker-sigterm branch June 11, 2019 11:38
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.

docker shutdown slow
2 participants