briandilley / ebs-deploy

Python based command line tools for managing Amazon Elastic Beanstalk applications.
MIT License
82 stars 29 forks source link

Zero-downtime deploy - configurable wait time for green status #12

Closed adamfeuer closed 10 years ago

adamfeuer commented 10 years ago

This pull request adds a --wait-time option to zdt_deploy to set the number of seconds the script waits for the environment to become green.

The default is 600 seconds; we are using the script to deploy Docker applications, and since the docker deploy pulls large filesystem layers from a remote Docker repo, the wait time needs to be a few more minutes. This change addresses that by letting the user specify the number of seconds to wait.

One other minor change in this pull request is that the polling time for status checks is changed from 5 seconds to 10 seconds to avoid throttling errors. Without this change, we sometimes get exceptions due to throttling errors during the time the environment is installing the new Docker filesystem.

briandilley commented 10 years ago

Too many format changes

briandilley commented 10 years ago

Hey adamfeuer, i'd really like to merge this in, but there are far too many formatting changes. anyway you could submit a new PR?

adamfeuer commented 10 years ago

Brian,

Hey, sorry - I was away from email on a meditation retreat. On my way back home now. :-)

Sure, I will make a new pull request and submit it this weekend.

Thanks for being willing to take these changes- we are using ebs-deploy to support our production services and it's really helping us.

-adam

On Friday, October 24, 2014, Brian notifications@github.com wrote:

Hey adamfeuer, i'd really like to merge this in, but there are far too many formatting changes. anyway you could submit a new PR?

— Reply to this email directly or view it on GitHub https://github.com/briandilley/ebs-deploy/pull/12#issuecomment-60434925.

Adam Feuer adam@adamfeuer.com

adamfeuer commented 10 years ago

The formatting changes came from PEP8 formatting the file in my IDE (PyCharm). I'll see if I can manually un-reformat it.

briandilley commented 10 years ago

cool, i welcome the changes - it's just tough when a large portion of the change is formatting.

briandilley commented 10 years ago

you know what, the formatting is fine - if you can merge in the latest master (resolving conflicts) then i'll merge this pull request in and do a release to pypi.

adamfeuer commented 10 years ago

It's no problem - the reformatting was my mistake. I understand the problem - it masks the real changes. I was just on autopilot (bad!) since we use PEP8 here where I work.

I undid most of the PEP8 formatting and pushed here - will you check this out now?

adamfeuer commented 10 years ago

Hmmm.... there seems to be a conflict that needs to be resolved, but I can't see it...

adamfeuer commented 10 years ago

I pulled the latest master branch, and merged my branch locally, and it merged ok. So I am not sure why GitHub is complaining that this branch can't be merged...

adamfeuer commented 10 years ago

I figured out where the merge conflict is, and can fix it. (I wasn't pulling my upstream master branch correctly.)

However, I am using this script in production and it has a problem terminating environments correctly. So I need to debug that before we can merge this. I'm going to close this for now. I'll re-do the pull request when I am confident my changes work.