feat(cmd/serve): add in default-shutdown-timeout flag to increase shutdown timeout on http server shutdown #1162
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tdown timeout on http server shutdown
|
|
|
Curious to hear suggestions on what may be an alternative approach to get a similar outcome & would be happy to try and make the changes needed for any feedback to help enable that |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1162 +/- ##
==========================================
- Coverage 77.90% 77.81% -0.09%
==========================================
Files 80 80
Lines 3929 4044 +115
==========================================
+ Hits 3061 3147 +86
- Misses 595 617 +22
- Partials 273 280 +7 ☔ View full report in Codecov by Sentry. |
aeneasr
reviewed
Jul 4, 2024
| @@ -32,4 +32,5 @@ func init() { | |||
|
|
|||
| serveCmd.PersistentFlags().Bool("disable-telemetry", false, "Disable anonymized telemetry reports - for more information please visit https://www.ory.sh/docs/ecosystem/sqa") | |||
| serveCmd.PersistentFlags().Bool("sqa-opt-out", false, "Disable anonymized telemetry reports - for more information please visit https://www.ory.sh/docs/ecosystem/sqa") | |||
| serveCmd.PersistentFlags().Int("default-shutdown-timeout", 5, "Set the default shutdown timeout in seconds for server shutdown when trapping SIGTERM and SIGINT") | |||
There was a problem hiding this comment.
In my view this should go into the config file instead
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Under the hood
ory/oathkeeperuses https://github.com/ory/graceful to shutdown the HTTP proxy. Thegracefulpackage provides a constant calledDefaultShutdownTimeoutwhich sets the timeout to be5seconds.We have an application that can take up to
60seconds to finish processing requests and we need a solution to be able to change this shutdown time in order for those requests to complete successfully.As a result, we need a way to configure
oathkeeperto bump up that timeout and the implementation has a flag that will be able to override the global variable set in thegracefulpackage. Although this does not seem like the most ideal solution it provides a way for us to change that value.Curious to hear suggestions on what may be an alternative approach to get a similar outcome & would be happy to try and make the changes needed for any feedback to help enable that.
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got the approval (please contact
security@ory.sh) from the maintainers to push
the changes.
works.