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

[WIP] New code generator based on code policy #1301

Closed
wants to merge 62 commits into from

Conversation

DrLuke
Copy link
Contributor

@DrLuke DrLuke commented Jan 27, 2019

Work in Progress, do not merge

Intro

The discussion in #1295 resulted in a desire to augment the code generation script. Having worked with it myself previously, I've decided to completely rewrite it to create a more stable and robust code generator.

Structure

It is split into 3 pieces:

  • Specification parser (specification.py and types.py)
  • Data generator (codedata.py and generator.py)
  • Code generation policy (policy.py)

Specification

The specification parser takes the JSON specification and parses it into python objects for further processing. It serves as nothing more than a code representation of the specification.

Generator

Using this information, the data generator generates objects with data relevant to generating code, like class-names and the properties. It also divides the properties and resources into modules named after the AWS service they belong to. It also detects Properties that aren't unique to a service, and moves them to the common.py module (currently only the the Tag property is affected by this).

Policy

Lastly, the policy takes this data and generates the actual code. With the policy you can define different syntaxes for code generation. For example you'd have different policies for python 2.7 vs 3.7.

What's next?

A maintainable way to add special requirements and validations for some classes is needed. I think the best way is to use the specification's name-spaced name (i.e. AWS::Lambda::Function.Code.S3Bucket to address the S3Bucket property of a lambda function code property). Validators could be defined with regexes, valid value ranges or some other common requirements that are shared across many properties. For cases requiring custom code an extra validators module could be maintained.

Also I would like to recommend moving away from the current format of adding properties to classes. Currently they are added using the props dict, from which the class fields are then populated at run-time. This is pretty unpythonic behavior and in my opinion unnecessary for generated code like this. Instead all the fields should be added directly by the generator. That way IDEs have a chance to suggest auto-completions, increasing overall usability of troposphere and allowing static code analysis (like type checking).

The goal is to achieve zero editing of generated code, and minimal editing of validation code with new versions.

How to run

  • Fetch a current specification (Single file JSON) and put it in project root.
  • mkdir -p build/3.7 build/2.7
  • python3 scripts/gen.py

The generated code will be output to build/2.7 and build/3.7.

Some unit tests are included but don't cover the new code entirely yet.

Conclusion

This code generator will allow to easily maintain code for different version of python, including python 2 until it's support officially ends with 2019. With some changes to how troposphere works it should be possible to make it a lot more pythonic than it currently is, increasing productivity for users while keeping the impact from changes minimal.

Some review and feedback would be greatly appreciated!

@markpeek
Copy link
Member

Thank you for the PR. I was able to take a quick look at it but likely will need more time looking at it. I wanted to give you my initial thoughts.

There is a lot I like in the work you put into this generator. Code cleanup, separation of concerns, unit tests, output different python versions, etc. As you may have seen, I created a PR #1300 which took a different approach to the same problem. For me, I was more focused on prototyping a model for the validation and overrides (that inevitably happen) between the spec/doc and real implementation in CloudFormation. I'd like to hear your thoughts on the yaml/_validators.py approach I took. Note: while I would like to see a regex or other approach, I think some of the validation will still require non-generated code.

Some quick issues that came up:

  • Need to push troposphere_gen/__init__.py
  • Instructions should include mkdir -p build/3.7 build/2.7
  • Like I commented, would like to hear thoughts on my way of including validators with yaml and mixins
  • Rather than bubble sort, use a tree and DFS to ensure dependencies are emitted first (my version does this).
  • Would need to understand how to push both 2.7 and 3.7 out to PyPI...never looked into that before.

And the bigger issue, I'd like to hear more on how you'd like to restructure the python classes to remove props. I was looking to make incremental changes to maintain compatibility as much as possible. Perhaps that means we make changes incrementally. For this aspect, it might be better to have a discussion in an issue rather than adding into this commit immediately. There's a lot I like about this commit and perhaps merging some of my PR ideas into it would give us a better base while maintaining compatibility with existing structure, usage, and validation the users expect.

@DrLuke
Copy link
Contributor Author

DrLuke commented Jan 27, 2019

Thank your for your positive feedback, it's much appreciated!

I think some of the validation will still require non-generated code.

This is most certainly inevitable

Rather than bubble sort, use a tree and DFS to ensure dependencies are emitted first (my version does this).

If we were to move away from the props dict and instead define fields in the __init__ method, this issue vanishes, as the python interpreter can resolve this (it parses classes first, then then methods). I'm not sure if this is also true for python 2, maybe the sorting function should be moved to the policy instead, as it would be implementation-dependent in that case. Bubblesort was just a quick way to get it working, I'll take a look at your approach.

Would need to understand how to push both 2.7 and 3.7 out to PyPI...never looked into that before.

Maybe move python 2 code to a separate package called troposphere2. As for delivering different python 3 versions from the same package: I think this can be done in the setup.py. It's just normal code after all. This will become necessary for python 3.4 (no type annotation) vs 3.5 (type annotations only in function signature) vs. 3.7 (full type annotations). I think python 3.4 support should be the minimum to aim at (EOL is expected for March 2019)

And the bigger issue, I'd like to hear more on how you'd like to restructure the python classes to remove props. I was looking to make incremental changes to maintain compatibility as much as possible.

As you suggested we should discuss this in a separate issue. For the time being any new generator should best stay compatible with the current code.

I will take a look at your prototype tomorrow.

@stevengorrell
Copy link
Contributor

I think I would rather see 3.5 as a minimum. March is already around the corner and given either solution will probably take a while to implement, I would rather see it bumped to a higher version to cut down on possible reworks. Just my $0.02

@markpeek
Copy link
Member

@stevengorrell are you implying 3.4 is going EOL in March? I tried to find an EOL schedule but couldn't find it immediately. Supporting 3.4+ has been pretty easy. I tried to add 3.7 (again) but there is a Python issue that causes unittests to fail.

@stevengorrell
Copy link
Contributor

stevengorrell commented Jan 29, 2019 via email

@markpeek markpeek added the Future label Feb 2, 2019
@DrLuke DrLuke closed this Nov 6, 2021
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.

3 participants