How to make all this more extendable?

[antonbabenko]

Thanks for spending your time helping with the reviewal of Python code in this project!

handler.py has function handler which is invoked by AWS Lambda via API Gateway.

Currently this project implements:

• [x] convert Cloudcraft json blueprint into modules.tf-schema
• [x] convert modules.tf-schema into Terragrunt code

I am working on adding support for extension or ways to expand this project, for example:

• [ ] convert modules.tf-schema into Terraform code (in addition to Terragrunt)
• [ ] support other formats as inputs (now only Cloudcraft json is supported)

While doing this I feel that I can do it and will spend a lot of time, so I am looking for some advice/help (in no specific order):

1. handler now has a linear flow, but it will not be so always. How can I split handler.py into several smaller files and include them conditionally (eg, generate/terraform.py and generate/terragrunt.py) instead of calling render_terraform_from_modulestf_config or render_terragrunt_from_modulestf_config? Does it make sense to add OOP / Proxy pattern, or is there another more pythoniс way?
2. Does it make sense to split handler.py into several smaller files at this point? Now the function generate_modulestf_config implements a single interface (Cloudcraft), but I want it to implement different also, and have smaller files.
3. How would you recommend to test this? I do run this with different arguments, but it is not automated more than that.

Please think about this and try to give me an advice for so called "the lowest hanging fruit".

Thank you!

[ashald]

Many things can be changed about this code and its organization, but since your already mentioned testing - it's a great place to start from! Also, would qualify for "lowest" hanging fruit.

First and foremost, I'd suggest introducing tox. It's widely used for test automation. You don't have to add any tests to do that - your 1st test environment could be running PEP8 validation. Making your code PEP8 compliant is the 1st step at making it "pythonic" and great.

Here is an example tox.ini that you can use in this case. You can start with it or you can research the topic and come up with your own.

[tox]
envlist = flake8
skipsdist = True

[testenv:flake8]
skip_install = True
deps = flake8
commands = flake8 handler.py

[flake8]
max-line-length = 99
count = True
statistics = True
show-source = True
max-complexity = 10

Unfortunately, flake8 cannot catch all the "violations" (for instance, imports). Generally speaking, it's great to read PEP8 at least once to get a general idea about it. IDK about your preferences, but using an IDE like PyCharm (there is a free version for OSS development) also helps a lot as it has really good built-in PEP8 style checks.

Once you're ready to add more tests - py.test is the most widely used testing framework.

[ashald]

Second, I'd suggest using a more explicit definition of supported Python version(s). There are many Python3 versions and they are quite different in terms of available feature. If you don't have any constraints, I recommend going with the latest one.

Generally speaking, it's better to avoid versions before 3.4. Staring from 3.5 you can use type-annotations that make code a lot easier to read/check/verify and starting from 3.6 you can rely on so-called f-strings that simplify formatting a lot.

If you will decide to support many versions, tox would help you to run tests against all of them.

[ashald]

IDK about your long-term plans for this code and how it's going to be distributed and used, but making your code a package usually helps a lot. You can go with native setuptools(https://packaging.python.org/tutorials/packaging-projects/#setup-py) or use "wrappers" like pbr (https://pypi.org/project/pbr/) or packit (https://pypi.org/project/packit/).

Disclaimer: I'm one of authors of packit and have built it at my current workplace. It's far from ideal, as well as Python packaging in general, but it simplifies a lot of things until you're really ready to get your hands dirty.

If your code is packaged then it can be uploaded to PyPI, pip install-ed and it's easier to define its dependencies. I'll submit a pull-request showing how it can be done with packit.

[ashald]

From there one, I'd suggest fixing general problems such us:

1) avoid code execution upon module load (json.load in MODULES definition) 2) avoid using globals 3) avoid shadowing builtins 4) close open files (prefer context manager style with with)

Once that is done I'd focus on using more idiomatic constructs and features of standard library. For instance: 1) tmpfile module instead of tmp_dir = "/tmp" 2) if test instead of if text is not None and len(text) 3) classes - they are commonly used in Python for "namespacing"

At this point code should prtty idiomatic and one can think about structuring it into several modules (read "files") or even packages (read "dirs").

[antonbabenko]

Thanks @Ashald for reviewing this. I really appreciate it all!

I will start with fixing general problems as you mentioned in this comment and look into adding tox and py.test.

Packaging&distribution is not a requirement for now, so I will skip this part.

[ashald]

Let me know if you want me to make another pass.

[antonbabenko]

I will need another review probably later next week when I do some more coding required for public demo at the conference :)

I will certainly let you know! Thanks for proposing.