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

Pass stop_timeout down to underlying http request #171

Merged
merged 1 commit into from
Jun 9, 2016
Merged

Conversation

intjonathan
Copy link
Contributor

When setting a custom stop_timeout, the value is passed to the docker API, but Excon won't wait any longer than 120 seconds for the response. This can lead to incorrect timeout errors if the container takes longer than that to stop.

This passes the docker stop_timeout to the Excon request so it will wait at least as long as docker will for the container to stop.

When setting a custom stop_timeout, the value is passed to the docker API, but Excon won't wait any longer than 120 seconds for the response. This can lead to incorrect timeout errors if the container takes longer than that to stop.

This passes the docker stop_timeout to the Excon request so it will wait at least as long as docker will for the container to stop.
@relistan relistan merged commit a4cea02 into master Jun 9, 2016
relistan added a commit that referenced this pull request Jun 9, 2016
intjonathan pushed a commit that referenced this pull request Jan 13, 2017
Passing identical timeouts to Docker and the Excon client resulted in
Excon giving up too soon when Docker was trying to kill timed-out containers.

For example, you may set a docker stop timeout of 30, expecting your
service to respond adequately to a KILL after 30 seconds. However,
your deploy would stop immediately after the 30 second timeout because
Excon would give up right away.

This adds 120 seconds to the given stop timeout, then passes that to
the Excon client. This should give the deploy plenty of time for
Docker to do its business even on sad servers.

Followup to #171
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.

2 participants