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

feat(cfn-include): the package cloudformation-include is now 'Developer Preview' #10436

Merged
merged 2 commits into from
Sep 18, 2020

Conversation

skinny85
Copy link
Contributor


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

@skinny85 skinny85 added the pr-linter/exempt-readme The PR linter will not require README changes label Sep 18, 2020
@skinny85 skinny85 self-assigned this Sep 18, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Sep 18, 2020
@skinny85 skinny85 added the pr-linter/exempt-test The PR linter will not require test changes label Sep 18, 2020
Copy link
Contributor

@NetaNir NetaNir left a comment

Choose a reason for hiding this comment

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

🥳

@mergify
Copy link
Contributor

mergify bot commented Sep 18, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 31f35b9
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Sep 18, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit d45a57c into aws:master Sep 18, 2020
@skinny85 skinny85 deleted the feat/cfn-include-dev-preview branch September 18, 2020 23:35
@alexpulver
Copy link
Contributor

Long awaited :). Thanks!

@skinny85
Copy link
Contributor Author

Long awaited :). Thanks!

@alexpulver our pleasure. If you have any feedback on the cloudformation-include library, please let us know!

@alexpulver
Copy link
Contributor

One thing I already noticed, is that adding cloudformation-include module to the project dependencies pulls in almost all (or all) other AWS CDK modules, resulting in ~200MB virtual environment size (in Python case). I assume this is by design, since these are required for casting the resources, is that correct?

P.S.: I understand that AWS CDK moves back to monolithic packaging design anyway in CDK v2, which should improve install time for all modules.

@skinny85
Copy link
Contributor Author

You're exactly correct @alexpulver - cloudformation-include depends on all CDK modules that define Cfn* resources (exactly because it needs access to the underlying Cfn* classes), which is most of them, and the CDK is moving to monolithic packaging, which will include the cloudformation-include module when that transition happens.

@alexpulver
Copy link
Contributor

Thanks :). Another question/feedback - I see that CfnInclude class creates a resource tree under the parent construct node. Would it make sense to just rely on the regular CDK escape hatches mechanism for CFN resource lookup and customization, without explicit CfnInclude.get_resource() calls?

image

@skinny85
Copy link
Contributor Author

That's more of an implementation detail. For example, things like Conditions, Outputs, Mappings, etc. can actually have the same names as Resources and Parameters in the template. Because of that, they're created under a different scope than this (you can even see those parent scopes in your screenshot, indexes 0 through 2 under children in CfnInclude).

Methods like get_resource, get_condition, get_output, get_mapping, etc. hide those details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants