-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
feat: implement graceful shutdown on sigterm #728
Conversation
@@ -16,7 +16,7 @@ COPY --from=build /usr/src/app/node_modules node_modules | |||
COPY --from=build /usr/src/app/dist dist | |||
COPY package.json ./ | |||
ENV PG_META_PORT=8080 | |||
CMD ["npm", "run", "start"] | |||
CMD ["node", "dist/server/server.js"] |
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.
What was wrong with npm start
(run
can be omitted here)?
It's one more place you've to remember to update dist/server/server.js
in case it's moved 👀
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.
npm run start
wasn't passing down signals from the parent process. I tried it initially with a barebone docker-compose.yml
.
version: '3'
services:
pgmeta:
build: .
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.
may be related to this issue npm/cli#6684
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.
🫠
Sweet! Would've thought this is built-in |
What kind of change does this PR introduce?
Bug fix #663
What is the new behavior?
Uses closes-with-grace package from fastify authors. Shuts down the server on the following events
Additional context
Tested with
docker-compose.yml