-
Notifications
You must be signed in to change notification settings - Fork 86
Support 'eb deploy ... --archive <ZIP|dir>' #535
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
Conversation
12b391e to
9a5d1ac
Compare
| # http://aws.amazon.com/apache2.0/ | ||
| # | ||
| # or in the "license" file accompanying this file. This file is | ||
| # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF |
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.
Minor thing but the description starts off by saying this is the environment name change but I think that's the other PR
| 'These are mutually exclusive methods for specifying application code.', | ||
| 'deploy.archivewithsource': 'You cannot use the "--archive" option with the "--source" option for environment updates. ' | ||
| 'These are mutually exclusive methods for specifying application code.', | ||
| 'deploy.archivemustbedirirzip': 'The "--archive" option requires a directory or ZIP file as an argument.', |
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.
nit: typo in archivemustbedirirzip
| else: | ||
| s3_bucket, s3_key = get_app_version_s3_location(app_name, version_label) | ||
|
|
||
| file_name = file_path = None, None |
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.
nit: i think this is more clear as file_name, file_path = None, None
| if self.archive and not self.app.pargs.region: | ||
| raise InvalidOptionsError(strings['deploy.archivewithoutregion']) |
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.
why is region required?
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.
It comes down to the EB CLI idiom that (almost) every eb command must be executed from within a (potentially Git-initialized) workspace. What defines a workspace is an .elasticbeanstalk directory with a config.yml file. The config.yml file is expected to contain references to the **region and application name that the workspace must be associated with.
With --archive, we are deviating from this in that we are letting eb deploy operate in non-eb init-ed workspaces as well. However, the workspace will then not have a .elasticbeanstalk/config.yml file to reference. To avoid this confusion, we force the customer to instruct that the region be selected. Incidentally, this is the behaviour of the AWS CLI when there's not default region in ~/.aws/config or IMDS doesn't exist or cannot tell the present container what the region is.
Another solution to avoid forcing selection of --region would have been to prompt the selection of the region interactively. I chose to not provide this option as eb deploy today does not prompt interactively (but several other commands do). We may choose to break this behaviour in the future although it wouldn't be a backwards-incompatible change in reality, just philosophically.
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.
Makes sense, and I think that's the right decision here
This commit adds support 'eb deploy ... --archive <ZIP|dir>' so that customers can point to arbitrary directories/ZIP files to deploy to existing environments. In the presence of the '--archive' argument, 'eb deploy' will ignore the .elasticbeanstalk/config.yml file and operate in its absence. The '--archive' argument will accept directories or ZIP files. If the argument is a directory, 'eb deploy' will ZIP the directory and save it in the '~/.ebartifacts/archives' directory. It will upload it to S3 and invoke EB::CreateApplicationVersion with the artifact. Like with the eb-deploy flow without '--archive', it will proceed to perform an EB::UpdateEnvironment and wait for the update to complete.
9a5d1ac to
aa5686e
Compare
Issue #, if available:
Related to #516. While the proposal in that issue is to allow passing in the application name, it is the environment name that makes more sense to pass in.
Description of changes:
This commit adds support
eb deploy ... --archive <ZIP|dir>so that customers can point to arbitrary directories/ZIP files to deploy to existing environments. In the presence of the--archiveargument,eb deploywill ignore the .elasticbeanstalk/config.yml file if it is present. If it is not present, it will operate in its absence. The--archiveargument will accept directories or ZIP files. If the argument is a directory,eb deploywill ZIP the directory and save it in the~/.ebartifacts/archivesdirectory. It will upload it to S3 and invoke EB::CreateApplicationVersion with the artifact. Like with the eb-deploy flow without--archive, it will proceed to perform an EB::UpdateEnvironment and wait for the update to complete.One limitation of the current approach is that it requires the environment to already exist. It would be nice to have
eb deployissue an environment trigger based on theeb create's sane defaults.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.