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

Making Troposphere More Pythonic #1320

Open
DrLuke opened this issue Feb 16, 2019 · 3 comments
Open

Making Troposphere More Pythonic #1320

DrLuke opened this issue Feb 16, 2019 · 3 comments

Comments

@DrLuke
Copy link
Contributor

DrLuke commented Feb 16, 2019

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:

  • __init__ doesn't contain all properties in the signature. Autocompletion thus won't show you all possible kwargs you could set.
  • The attributes are generated at run-time, not statically. Again, autocompletion is not able to suggest you attributes names.
  • Validation has to be added manually

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.

@DrLuke DrLuke changed the title Making Troposphere more pythonic Making Troposphere More Pythonic Feb 16, 2019
@MacHu-GWU
Copy link

MacHu-GWU commented Jul 8, 2019

@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
Copy link
Contributor Author

DrLuke commented Jul 11, 2019

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

@MacHu-GWU
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants