Skip to content

design: sam deploy also packages built artifacts #1521

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

Merged
merged 3 commits into from
Nov 15, 2019

Conversation

sriram-mv
Copy link
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@jfuss jfuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally this looks good. Just a couple questions/nits.


`sam deploy --stack-name sam-package-on-deploy --capabilities CAPABILITY_IAM --s3-bucket sam-package-bucket`

There is no explicit need to pass in a `--template-file` parameter, if one is not passed in it to defaults to trying to find the template.yaml that was generated by `sam build` which is located at `.aws-sam/build/template.yaml`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If sam build was never run what happens? Do we pick up the default template from the cwd or fail? All of our other commands pick up the default but could see a case to fail and leave the door open for a deploy that builds, packages, and deploys. Would be worth describing what will happen here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we pick up the default template. Can describe that case.

Future of `sam package`?
---------------------

* `sam package` will continue to exist in the state it is today and will continue to be improved upon separately.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for clarification, if we update package this new deploy flow will also get those updates?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct. all changes will go into PackageContext instead, which deploy will reap the benefits of.

CLI Changes
-----------

* Add new arguments `--metadata`, `--use-json` and modify existing `--template-file` to look for a default `template.yaml` that exists under `.aws-sam/build/`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What value does --use-json provide in the deploy experience?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a given template file is above 51,200 bytes. that corresponding template also gets uploaded to an s3 bucket, if --use-json is provided, that uploaded template_file will be in the json format.

* If a pre-packaged template is specified, an attempt to package does not change the template and the same template is used for deploy.
* The parameters that share the same name across package and deploy are collapsed together. eg: `--kms-key-id` , if a kms-key-id is specified that same key is used across both packaging and deploy purposes.

`.samrc` Changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update the template in the sam config so this gets reflected in new designs going forward. Just noticing this now but not really apart of this pr directly.

Documentation Changes
=====================
* Required nature of `--template-file` parameter has a series of defaults that are looked at during `sam deploy` similair to `sam package`.
* If `--template-file` points to a non-packaged template-file, `--s3-bucket` becomes required to be able to effectively package and deploy in one command using `sam deploy`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also --template right?

@jfuss jfuss added area/deploy sam deploy command area/package sam package command type/design labels Nov 12, 2019

Parameters that dont need to be added.

* `--output-template-file`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we print this with --debug and document this so people can use this when debugging?

@sriram-mv sriram-mv changed the base branch from develop to release-v0.32.0 November 13, 2019 19:08
@sanathkr sanathkr merged commit ae74f15 into aws:release-v0.32.0 Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/deploy sam deploy command area/package sam package command type/design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants