-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Modify CloudFormation resource provider scaffolding to use "live" schemas #13513
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
Conversation
Test Results - Preflight, Unit22 978 tests - 23 21 136 ✅ - 22 6m 5s ⏱️ -18s Results for commit c232f45. ± Comparison against base commit ca6b22b. This pull request removes 23 tests.♻️ This comment has been updated with latest results. |
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 31m 5s ⏱️ - 1h 24m 18s Results for commit c232f45. ± Comparison against base commit ca6b22b. This pull request removes 4557 tests.♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files ± 0 5 suites ±0 43m 17s ⏱️ - 1h 52m 10s Results for commit c232f45. ± Comparison against base commit ca6b22b. This pull request removes 4913 tests.♻️ This comment has been updated with latest results. |
Test Results - Alternative Providers583 tests - 880 325 ✅ - 557 17m 57s ⏱️ - 15m 14s Results for commit c232f45. ± Comparison against base commit ca6b22b. This pull request removes 880 tests.♻️ This comment has been updated with latest results. |
simonrw
left a comment
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 looks like a very sensible change, thanks!
Next step will be to render the scaffolding to a separate directory and update all resource providers to subclass the provider so we can separate autogenerated and manually written code 👍
|
|
||
| def schema(self, type_name: ResourceName) -> ResourceSchema: | ||
| """ | ||
| Given a CloudFormation resource type name (e.g. AWS::S3::Bucket), return the resource schema as dict. |
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.
nit: strictly the argument is a ResourceName which can be created from a str with ResourceName.from_name(type_name)
Will do. What are your thoughts on how best to do that? I have a few ideas:
Sounds like you're suggesting (2) + (3)? |
I am siding with 2 and 3 because we can scaffold the base class which can include the Also on the topic of types it might be nice to consolidate the types for a service in the future if possible so we are not duplicating common type definitions. I don't know how much value there is in this (ie how many resources share identical types to other resources in the same service) but I'd like to find out! |
Motivation
To improve our CloudFormation resource providers, this change makes it easier to scaffold new resource types.
Changes
The following things have changed:
.zipfile.cloudformationdirectory).--proflag wasn't always been obeyed.Some future changes (not included here):
Tests
Related