cloudtools / troposphere

troposphere - Python library to create AWS CloudFormation descriptions
BSD 2-Clause "Simplified" License
4.92k stars 1.45k forks source link

Making Troposphere More Pythonic #1320

Open DrLuke opened 5 years ago

DrLuke commented 5 years ago

I would like to propose an update to the auto-generated classes of troposphere, specifically to improve their usability in modern IDEs. Additionally this allows for easy value validation, which can be defined during code generation, removing the need to manually edit generated code after the fact.

These changes are part of PR #1301 and related to issue #1295.

The current state

Currently the following class code is generated (python 2):

class MethodResponse(AWSProperty):

    props = {
        "ResponseModels": (dict, False),
        "ResponseParameters": (dict, False),
        "StatusCode": (basestring, True)
    }

Subproperties are added via the class-dict props. It tells the type this property should have, as well as if it's required. Using this data, the init method can read the **kwargs to populate the values or you can set the attributes after object initialization.

The problems with this are:

My proposal

As I am currently rewriting the generator in python 3, it only make sense to rework this into more pythonic class code. Instead of moving the class instantiation to run-time, I do all the heavy work at generation-time, producing static classes that don't require voodoo going on behind the curtains.

class MethodResponse(AWSProperty):
    props = {
        'response_models': Dict[str, str],
        'response_parameters': Dict[str, bool],
        'status_code': str,
    }

    def __init__(self,
                 response_models: Dict[str, str] = None,
                 response_parameters: Dict[str, bool] = None,
                 status_code: str = None,
                 ):
        self._response_models: Dict[str, str] = None
        self.response_models = response_models
        self._response_parameters: Dict[str, bool] = None
        self.response_parameters = response_parameters
        self._status_code: str = None
        self.status_code = status_code

    @property
    def response_models(self) -> Dict[str, str]:
        return self._response_models

    @response_models.setter
    def response_models(self, value: Dict[str, str]) -> None:
        if value is None:
            self._response_models = None
            return
        if not isinstance(value, dict):
            raise ValueError("'response_models' must be of type 'dict' (is: '%s')" % type(value))
        for k, v in value.items():
            if not isinstance(k, str):
                raise ValueError("response_models map-keys must be of type 'str' (is: '%s')" % type(k))
            if not isinstance(v, str):
                raise ValueError("response_models map-values must be of type 'str' (is: '%s')" % type(v))
        self._response_models = value

    @property
    def response_parameters(self) -> Dict[str, bool]:
        return self._response_parameters

    @response_parameters.setter
    def response_parameters(self, value: Dict[str, bool]) -> None:
        if value is None:
            self._response_parameters = None
            return
        if not isinstance(value, dict):
            raise ValueError("'response_parameters' must be of type 'dict' (is: '%s')" % type(value))
        for k, v in value.items():
            if not isinstance(k, str):
                raise ValueError("response_parameters map-keys must be of type 'str' (is: '%s')" % type(k))
            if not isinstance(v, bool):
                raise ValueError("response_parameters map-values must be of type 'bool' (is: '%s')" % type(v))
            validators.regex(k, self, must_match="method\.response\.header\.[\x21-\x39\x3b-\x7e]+$")
        self._response_parameters = value

    @property
    def status_code(self) -> str:
        return self._status_code

    @status_code.setter
    def status_code(self, value: str) -> None:
        if value is None:
            self._status_code = None
            return
        if not isinstance(value, str):
            raise ValueError("status_code must be of type 'str' (is: '%s')" % type(value))
        validators.regex(value, self, must_match="\d\d\d")
        self._status_code = value

The generated code becomes a whole lot longer, but bear with me.

Properties

The generator creates properties for every subproperty. This means we get a getter and setter function, where validation can happen. This also gives autocompleters the opportunity to suggest attributes while writing code, making the use of the classes easier. Instead of constantly checking the documentation for the correct name of each property, you let the autocompleter do the work.

Validation

Each setter has built-in type checking. For lists and dicts each value is checked for the correct type, additionally for dicts the keys are checked if they're strings. Additionally a validator can be specified for each subproperty, allowing actual content checking. If the validation fails, an exception is thrown.

Validators are defined with a JSON-File, with the property name as the key. For each subproperty the function to be called is specified, as well as any number of kwargs to be passed to it. Additionally self is also passed to the validator, in case a value's validity is dependent on other values.

Example validator definition for the above code:

{
    "PropertyTypes": {
        "AWS::ApiGateway::Method.MethodResponse": {
            "Properties": {
                "ResponseParameters": {
                    "Validator": "Map",
                    "ValidatorKey": {
                        "Validator": "regex",
                        "must_match": "method\\.response\\.header\\.[\\x21-\\x39\\x3b-\\x7e]+$"
                    }
                },
                "StatusCode": {
                    "Validator": "regex",
                    "must_match": "\\d\\d\\d"
                }
            }
        }
    },
    "ResourceTypes": {}
}

Validators are imported with from troposphere.validators import *. This allows developers to either use generic validators like regex or easily add validators tailored for a single property. Properties are addressed in exactly the same format as in the specification JSON to reduce any confusion as much as possible, things already are complicated enough as they are. Passing parameters as kwargs also makes any custom formatting for each validator unnecessary, further reducing complexity in the generator.

Feedback

I'd love to get some feedback and constructive criticism for this. I understand it's a quite significant change to the inner workings of troposphere, but this is a good opportunity to improve it.

MacHu-GWU commented 5 years ago

@DrLuke , @xyvark , @grkking, @markpeek Please take a look at my project https://github.com/MacHu-GWU/troposphere_mate-project

It built a thin wrapper layer on top of troposphere using attrs. It is exactly the pythonic implementation with nicer API. And type hint is enabled with type hint markup described https://www.jetbrains.com/help/pycharm/type-hinting-in-product.html.

And it comes with a feature called auto-reference/associate. You don't need to remember the detailed properties and the reference syntax to reference each other, you just use this, it automatically does the work for you:

associate(lambda_func, iam_role)
associate(lambda_func, subnet1)
associate(lambda_func, subnet2)
associate(lambda_func, security_group)

So it gives you auto suggestions on type and properties name.

DrLuke commented 5 years ago

@MacHu-GWU Please don't hijack issues like this just to advertise your own project. It's off-topic.

MacHu-GWU commented 5 years ago

@DrLuke OK, I mean this is the easiest way I can think of to make it more pythonic.