feat(core) Add resource type and properties for all CfnResource constructs to tree.json by shivlaks · Pull Request #4894 · aws/aws-cdk

shivlaks · 2019-11-07T09:02:58Z

Modifies the children node from an array to an object with each child object keyed on its id.
Adds an interface IInspectable that constructs can optionally implement to contribute attributes into tree.json.
For extensibility, the inspect method accepts an ITreeInspector. This way, new capabilities can be added without changing the contract of inspectable constructs.
Generated classes for Cfn resources implement IInspectable and contribute their resource type and props

Supercedes #4562

TODO:

Token Support

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

…ructs to tree.json Modifies the children node from an array to an object with each child object keyed on its id. Adds an interface `IInspectable` that constructs can optionally implement to contribute attributes into `tree.json`. For extensibility, the `inspect` method accepts an `ITreeInspector`. This way, new capabilities can be added without changing the contract of inspectable constructs. Generated classes for Cfn resources implement `IInspectable` and share their resource type and props

mergify · 2019-11-07T09:03:36Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

packages/@aws-cdk/core/lib/tree.ts

eladb · 2019-11-07T09:15:54Z

packages/@aws-cdk/core/test/private/test.tree-metadata.ts

+      }
+
+      public inspect(inspector: ITreeInspector) {
+        inspector.attribute('aws:cdk:cloudformation:type', 'CDK::UnitTest::MyCfnResource');


weren't these supposed to be added by the CfnResource base class?

eladb · 2019-11-07T09:17:11Z

tools/cfn2ts/lib/codegen.ts

+   * }
+   *
+   */
+  private emitTreeAttributes(resource: genspec.CodeName): void {


why not implement this in CfnResource?

I initially started out trying that, but the main reason for the shift was the properties, each generated class has it's own implementation of cfnProperties which is being emitted here.

Is there another way past that I haven't considered?

Ok let’s leave it like this for now

aws-cdk-automation · 2019-11-07T09:24:20Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2019-11-07T10:42:16Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2019-11-07T11:54:47Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

packages/@aws-cdk/core/lib/private/tree-metadata.ts

packages/@aws-cdk/core/lib/tree.ts

eladb · 2019-11-11T07:17:53Z

tools/cfn2ts/lib/codegen.ts

+   * }
+   *
+   */
+  private emitTreeAttributes(resource: genspec.CodeName): void {


Ok let’s leave it like this for now

aws-cdk-automation · 2019-11-11T07:29:32Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2019-11-11T09:12:30Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2019-11-11T10:23:56Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

shivlaks added 2 commits November 7, 2019 00:47

formatting

cf0d7b6

shivlaks requested review from eladb and rix0rrr November 7, 2019 09:02

eladb suggested changes Nov 7, 2019

View reviewed changes

renamed attribute to addAttribute

a39e7b5

change type of attribute bag to take in any K-V pair

0a7b0a9

rix0rrr reviewed Nov 8, 2019

View reviewed changes

packages/@aws-cdk/core/lib/private/tree-metadata.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/private/tree-metadata.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/tree.ts Outdated Show resolved Hide resolved

SomayaB added the contribution/core This is a PR that came from AWS. label Nov 8, 2019

SomayaB assigned eladb Nov 8, 2019

moving to concrete class for inspecting tree rather than an interface

42a0ddd

eladb approved these changes Nov 11, 2019

View reviewed changes

Merge branch 'master' into shivlaks/construct-tree

9bf9bce

shivlaks added the pr/do-not-merge This PR should not be merged at this time. label Nov 11, 2019

Merge branch 'master' into shivlaks/construct-tree

379d675

shivlaks removed the pr/do-not-merge This PR should not be merged at this time. label Nov 11, 2019

shivlaks merged commit 4037155 into master Nov 11, 2019

shivlaks deleted the shivlaks/construct-tree branch November 11, 2019 10:29

Conversation

shivlaks commented Nov 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mergify bot commented Nov 7, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eladb Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

eladb Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

shivlaks Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

eladb Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Nov 7, 2019

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Nov 7, 2019

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Nov 7, 2019

AWS CodeBuild CI Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eladb Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Nov 11, 2019

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Nov 11, 2019

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Nov 11, 2019

AWS CodeBuild CI Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shivlaks commented Nov 7, 2019 •

edited

Loading