-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(toolkit): introduce the concept of auto-deployed Stacks #2046
Conversation
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.
- "ephemeral" does feel like the right terminology.
- I don't see how we can remove them from
cdk synth
(you'll need the template even in the pipeline case).
Eventually, I'd say this is basically a flag that allows you to decide if this stack is directly deployed by the cdk deploy
, so I'd just call this property deploy
and have it default to true
. Then print a little indication next to it in cdk ls
and automatically filter it from cdk deploy
(but allow to deploy it if it's explicitly specified).
How about calling it The behaviour should be only to exclude from Should we add an |
I like “autoDeploy” |
Like I said in the description: these Stacks do not appear when you do Does this make sense? |
Also, the name 'ephemeral' is sooo much cooler than 'autoDeploy' in my opinion... but I won't insist. |
Agreed :) but ephemeral also implies that it will go away after a while, and that is not true. |
By the way I just piled on without reading the code, sorry!! This completely does what I expected it to do, except for the name. And the build error. |
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.
Provisory approval modulo rename :)
e0da41f
to
f3a9116
Compare
Changed the name from |
@@ -354,6 +354,7 @@ function renderLegacyStacks(artifacts: { [id: string]: cxapi.Artifact }, store: | |||
environment: { name: artifact.environment.substr('aws://'.length), account: match[1], region: match[2] }, | |||
template, | |||
metadata: artifact.metadata || {}, | |||
autoDeploy: artifact.autoDeploy, |
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.
this should also rendered in the non-legacy artifact or we will lose this feature once we deprecate this
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.
Where does the code for that live? That's the only place I could find this logic...
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.
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.
Answered by Elad offline... https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/cdk/lib/stack.ts#L472 .
The PR already includes a change in that method.
A Stack that is not auto-deployed is meant to be deployed outside the context of the `cdk deploy` command - for example, in a CodePipeline. These Stacks do not appear when running `cdk synth` or `cdk deploy`, unless you explicitly filter for them. This is useful when modeling things like Lambda in CodePipeline, where the main deployment needs to happen in the Pipeline, but you might want to test things locally before pushing it to the Pipeline.
f3a9116
to
8b5e0e2
Compare
Rebased to resolve some conflicts. |
A Stack that is not auto-deployed is meant to be deployed outside the context of the `cdk deploy` command - for example, in a CodePipeline. These Stacks do not appear when running `cdk synth` or `cdk deploy`, unless you explicitly filter for them. This is useful when modeling things like Lambda in CodePipeline, where the main deployment needs to happen in the Pipeline, but you might want to test things locally before pushing it to the Pipeline.
A Stack that is not auto-deployed is meant to be deployed outside the context of the
cdk deploy
command -for example, in a CodePipeline.
These Stacks do not appear when running
cdk synth
orcdk deploy
,unless you explicitly filter for them.
This is useful when modeling things like Lambda in CodePipeline,
where the main deployment needs to happen in the Pipeline,
but you might want to test things locally before pushing it to the Pipeline.
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.