-
-
Notifications
You must be signed in to change notification settings - Fork 100
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: graceful shutdown timeout #2422
feat: graceful shutdown timeout #2422
Conversation
set default option to 0ms Related issue: nestjs#2421
lib/graceful-shutdown-timeout/graceful-shutdown-timeout.service.ts
Outdated
Show resolved
Hide resolved
lib/graceful-shutdown-timeout/graceful-shutdown-timeout.service.ts
Outdated
Show resolved
Hide resolved
Thanks a lot @Lp-Francois well done! |
Co-authored-by: Livio Brunner <[email protected]>
Suggestion from @BrunnerLivio during PR review Co-authored-by: Livio Brunner <[email protected]>
Suggestion from @BrunnerLivio during PR review Co-authored-by: Livio Brunner <[email protected]>
Suggestion from @BrunnerLivio during PR review Co-authored-by: Livio Brunner <[email protected]>
…rvice Suggestion from @BrunnerLivio during PR review Co-authored-by: Livio Brunner <[email protected]>
Suggestion from @BrunnerLivio during PR review
Requested changes done ✅ @BrunnerLivio |
Great job, about to release this! |
Nice thanks ✌️ |
Moved the documentation from the README.md to the NestJS docs. |
this.logger.log( | ||
`Awaiting ${this.gracefulShutdownTimeoutMs}ms before shutdown`, | ||
); | ||
await sleep(this.gracefulShutdownTimeoutMs); |
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.
@BrunnerLivio you can just:
import { setTimeout } from 'node:timers/promises';
await setTimeout(this.gracefulShutdownTimeoutMs);
see: https://nodejs.org/api/timers.html#timerspromisessettimeoutdelay-value-options
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.
@BrunnerLivio sorry, wrong mention... the author of the PR is @Lp-Francois
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.
TIL, didn't know this API exist :)
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #2421
What is the new behavior?
Add a new option
gracefulShutdownTimeoutMs
that adds a delay to the application shutdown if specified.Does this PR introduce a breaking change?
Other information