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

Validate objects when initialized #2032

Open
markpeek opened this issue Apr 4, 2022 · 2 comments
Open

Validate objects when initialized #2032

markpeek opened this issue Apr 4, 2022 · 2 comments

Comments

@markpeek
Copy link
Member

markpeek commented Apr 4, 2022

Opening for @MattJaccino based on questions in another issue.

Not specific to WAFv2, but one thing that did strike me as odd was that resource objects can be successfully initialized without all of the required properties being given. I thought I remembered it erroring out in situations like that, or am I missing something. ?

I thought with a property defined as

class SomeProperty(AWSProperty):
    props: PropsDictType = {
        "Prop1": (TYPE, True),
        "Prop2": (TYPE, False),
    }

if you tried to initialize without defining Prop1, it would throw some kind of exception. It seems like you can initialize resource objects without including all of the required properties, or with none at all.

Was this always the case, or did something change? If not, could that potentially be a feature request?

@markpeek
Copy link
Member Author

markpeek commented Apr 4, 2022

This has always been the case due to the initial design of troposphere allowing for a more getter/setter interface:

>>> instance = ec2.Instance("myinstance")
>>> instance.ImageId = "ami-951945d0"
>>> instance.InstanceType = "t1.micro"
>>> t.add_resource(instance)

But this does bring up the point on whether validation could be done at object creation or when added to a template.

@MattJaccino
Copy link

Hi Mark,

I looked through the code and found there is a method of the BaseAWSObject called .validate(), but there is no code to run

    def validate(self) -> None:
        pass

but there is a different method called ._validate_props()

    def _validate_props(self) -> None:
        for k, (_, required) in self.props.items():
            if required and k not in self.properties:
                rtype = getattr(self, "resource_type", type(self))
                title = getattr(self, "title")
                msg = "Resource %s required in type %s" % (k, rtype)
                if title:
                    msg += " (title: %s)" % title
                raise ValueError(msg)

I see the validation bool defaults to True, so that those methods get run when converting to a dict, so maybe that .validate() method should call the validation methods for properties and title (if applicable), or maybe just for the properties.

I don't know, just an idea. Will work with the ._validate_props() method for now :)

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