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

Add a resolve to ArtifactPath #571

Merged
merged 2 commits into from
Aug 15, 2018
Merged

Add a resolve to ArtifactPath #571

merged 2 commits into from
Aug 15, 2018

Conversation

mindstorms6
Copy link
Contributor

When using artifact path for template configuration, the CDK would get upset.
This just adds a resolve that falls back to that location.

Fixes #570

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

When using artifact path for template configuration, the CDK would get upset.
This just adds a resolve that falls back to that location.

Fixes aws#570
@mindstorms6 mindstorms6 requested a review from eladb August 15, 2018 00:31
@@ -63,6 +63,7 @@ export = {
changeSetName,
role: changeSetExecRole,
templatePath: new ArtifactPath(buildAction.artifact!, 'template.yaml'),
templateConfiguration: new ArtifactPath(buildAction.artifact!, 'templateConfig.json')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is templateConfiguration always supposed to be an artifact path? Why not just change it's type to ArtifactPath and dereference .location in the L2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I'm aware - yes. Will try that instead.

However and maybe I'm being silly - would we ever not want to do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean implement resolve - ideally no... It's a cheap trick that we are trying to avoid at the L2 surface area

@mindstorms6 mindstorms6 merged commit f0e7bb5 into aws:master Aug 15, 2018
rix0rrr added a commit that referenced this pull request Aug 15, 2018
### Features

* __@aws-cdk/cdk__: Tokens can now be transparently embedded into
  strings and encoded into JSON without losing their semantics. This
  makes it possible to treat late-bound (deploy-time) values as if they
  were regular strings ([@rix0rrr] in
  [#518](#518)).
* __@aws-cdk/aws-s3__: add support for bucket notifications to Lambda,
  SNS, and SQS targets ([@eladb] in
  [#201](#201),
  [#560](#560),
  [#561](#561),
  [#564](#564))
* __@aws-cdk/cdk__: non-alphanumeric characters can now be used as
  construct identifiers ([@eladb] in
  [#556](#556))
* __@aws-cdk/aws-iam__: add support for `maxSessionDuration` for Roles
  ([@eladb] in [#545](#545)).

### Changes

* __@aws-cdk/aws-lambda__ (_**BREAKING**_): most classes renamed to be
  shorter and more in line with official service naming (`Lambda`
  renamed to `Function` or ommitted) ([@eladb] in
  [#550](#550))
* __@aws-cdk/aws-codepipeline__ (_**BREAKING**_): move all CodePipeline
  actions from `@aws-cdk/aws-xxx-codepipeline` packages into the regular
  `@aws-cdk/aws-xxx` service packages ([@skinny85] in
  [#459](#459)).
* __@aws-cdk/aws-custom-resources__ (_**BREAKING**_): package was
  removed, and the Custom Resource construct added to the
  __@aws-cdk/aws-cloudformation__ package ([@rix0rrr] in
  [#513](#513))

### Fixes

* __@aws-cdk/aws-lambda__: Lambdas that are triggered by CloudWatch
  Events now show up in the console, and can only be triggered the
  indicated Event Rule. _**BREAKING**_ for middleware writers (as this
  introduces an API change), but transparent to regular consumers
  ([@eladb] in [#558](#558))
* __@aws-cdk/aws-codecommit__: fix a bug where `pollForSourceChanges`
  could not be set to `false` ([@maciejwalkowiak] in
  [#534](#534))
* __aws-cdk__: don't fail if the `~/.aws/credentials` file is missing
  ([@RomainMuller] in
  [#541](#541))
* __@aws-cdk/aws-cloudformation__: fix a bug in the CodePipeline actions
  to correctly support TemplateConfiguration ([@mindstorms6] in
  [#571](#571)).
* __@aws-cdk/aws-cloudformation__: fix a bug in the CodePipeline actions
  to correctly support ParameterOverrides ([@mindstorms6] in
  [#574](#574)).

### Known Issues

* `cdk init` will try to init a `git` repository and fail if no global
  `user.name` and `user.email` have been configured.
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.

Can't use a ArtifactPath for template configuration in CloudFormation + CodePipeline
2 participants