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

Package and deploy in one command #1532

Merged
merged 5 commits into from Nov 15, 2019

Conversation

sriram-mv
Copy link
Contributor

  • if the template is left unspecified, built artifacts are chosen and s3 bucket is optionally needed to deploy in one command.

  • existing workflows to package and deploy do not break.

  • Write Design Document (Do I need to write a design document?)

  • Write unit tests

  • Write/update functional tests

  • Write/update integration tests

  • make pr passes

  • Write documentation

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

samcli/commands/deploy/command.py Show resolved Hide resolved
samcli/commands/deploy/command.py Show resolved Hide resolved
@@ -96,6 +99,14 @@
"changes to be made to the stack. The default behavior is to return a"
"non-zero exit code.",
)
@click.option(
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we talked about this before, why is this explicitly required? can't we default to using JSON for the intermediate template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, we can. user had the option with package, do we gain anything by removing it from deploy?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no obvious user benefit of this flag in sam deploy. Why expose then?

region=region,
profile=profile,
) as package_context:
package_context.run()
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you packaging always? This would be a breaking change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. its an attempt to package, if the codeuri's already contain packaged contents, we do not repackage.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is skirting the lines of a breaking change. Is there any other mechanism to create two clear code paths, one for previous experience and other for new one?

And, this relies on the fact that sam package is idempotent. That's an undocumented assumption of sam package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1533. Added this question here.

samcli/commands/package/exceptions.py Show resolved Hide resolved
samcli/commands/package/package_context.py Show resolved Hide resolved
Copy link
Contributor Author

@sriram-mv sriram-mv left a comment

Choose a reason for hiding this comment

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

Replied to comments.

- `samcli/commands/deploy/command.py` has high number of local
  variables, because of the nature of command, it has high number
  of arguments.
Copy link
Contributor Author

@sriram-mv sriram-mv left a comment

Choose a reason for hiding this comment

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

added to list.

@sriram-mv sriram-mv merged commit 7d360ad into aws:release-v0.32.0 Nov 15, 2019
sriram-mv added a commit that referenced this pull request Nov 20, 2019
* feat: beginnings of package and deploy together

* fix: plumb through configuration

- need to add additional parameters over from package.

* fix: conditional check to print text on package during deploy

* fix: exceptions for `package`

* lint: local variables rule

- `samcli/commands/deploy/command.py` has high number of local
  variables, because of the nature of command, it has high number
  of arguments.
sriram-mv added a commit that referenced this pull request Nov 23, 2019
* feat: beginnings of package and deploy together

* fix: plumb through configuration

- need to add additional parameters over from package.

* fix: conditional check to print text on package during deploy

* fix: exceptions for `package`

* lint: local variables rule

- `samcli/commands/deploy/command.py` has high number of local
  variables, because of the nature of command, it has high number
  of arguments.
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.

None yet

2 participants