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

Prototype new auto-generation via the AWS resource specification file #1300

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

markpeek
Copy link
Member

DO NOT MERGE THIS CHANGE - this is a WIP

As mentioned in #1295, I'm prototyping ways to better auto-generate troposphere files from the AWS resource specification. This should then allow for more rapid turnaround of new change from the CloudFormation team.

This PR is a quick first pass at improving troposphere python files using the AWS resource specification file. The base file for each service say, foo.py, will be auto-generated. There is an override file, foo.yaml, to include validation and other changes into the base file. The validation routines are now moved into foo_validators.py as either class mixins or property validation functions.

The main change is scripts/gen.py to improve the auto-generated files. The two files that are auto-generated are troposphere/batch.py and troposphere/s3.py with corresponding yaml and validator files. Note: some of the S3 changes are breaking changes due to class naming differences.

make spec2
python scripts/gen.py --name batch CloudFormationResourceSpecification.json > troposphere/batch.py
python scripts/gen.py --name s3 CloudFormationResourceSpecification.json > troposphere/s3.py

I'd like to get feedback on the general direction of this technique (note: I know the code needs cleanup and handle some edge cases). @phobologic @axelpavageau @cmmeyer

First pass at improving troposphere python files using the AWS
resource specification file. The base file for each service say, foo.py,
will be auto-generated. There is an override file, foo.yaml, to include
validation and other changes into the base file. The validation routines
are now moved into foo_validators.py as either class mixins or property
validation functions.
Copy link
Member

@phobologic phobologic left a comment

Choose a reason for hiding this comment

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

I like it - small in-line comment, but so far it looks great.

LogDeliveryWrite = "LogDeliveryWrite"
classes:
CorsRules:
MaxAge:
Copy link
Member

Choose a reason for hiding this comment

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

Hmm - should these maybe be a sub key of something like Properties: or something like that, just to avoid collisions? I don't think i can think of any right now, so not really a strong opinion, just curious if you thought about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

That likely is a good change. One thing I might put in there is something to allow renaming the class which we've had to do on occasion. Also, you may have noticed there are breaking changes due to renamed resource/properties which could be mapped for backward compatibility via either a rename or alias.

Copy link
Member Author

Choose a reason for hiding this comment

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

@phobologic also take a look at #1301.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ugh...should have thought about adding this to the above comment...sorry for the spam.

Look at both this and the other WIP with respect to the impact on stacker. Be good to understand those needs in case we change Python versions and/or how properties are used.

@markpeek markpeek added the Future label Feb 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants