cloudtools / troposphere

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

Expanding one_of validator #2011

Closed JohnPreston closed 2 years ago

JohnPreston commented 2 years ago
JohnPreston commented 2 years ago

@markpeek To add for 4.0 ? Would have preferred to test against troposphere.AWSHelperFn class but then gets cyclic import. Admittedly my own fault to set that on the ECS RuntimePlatform without testing it with a Fn:: at the time :-1:

markpeek commented 2 years ago

@JohnPreston the cyclic import can be worked around by importing AWSHelperFn within the function itself. There are examples in that same file.

def sample():
    from .. import AWSHelperFn
    pass
JohnPreston commented 2 years ago

@markpeek Okay, cool. Not sure where we were standing on include within functions. I mean, I don't even know for myself ^^ Should I update accordingly ?

markpeek commented 2 years ago

I think the change you might be wanting is something like below. Feel free to make the change in your code, test, and push an update.

def one_of(class_name, properties, property, conditionals):
    from .. import AWSHelperFn

    if isinstance(properties.get(property), AWSHelperFn):
        return

    if properties.get(property) not in conditionals:
...
JohnPreston commented 2 years ago

done @markpeek edit: I happen to use the other resource types that use one_of and it did not seem to have a negative impact on these

markpeek commented 2 years ago

I pulled your changes, split the commits into two (ecs typo and validator fix), tweaked slightly, and pushed manually.

Thanks for the fixes @JohnPreston!

markpeek commented 2 years ago

Oops, let's try the link to the changes this way 5447948786e92de312d3e8586e0139ccb6f8beda and 75db1661e17e7d6c36c53d269d999774a0a30e45

JohnPreston commented 2 years ago

Thanks @markpeek I will split the commits next time :+1: