search

cloudtools / troposphere

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

Fix __eq__ protocol #2197

Closed ndparker closed 8 months ago

ndparker commented 8 months ago

Hello,

I'm maintaining a deployment tool based on troposphere and the latest update (4.5.0) failed one of my tests. This PR is supposed to fix the issue.

If it is not known how to compare objects, __eq__ should return NotImplemented instead of False. That way the right side object of the comparison might possibly take over. In my specific case the other object is a mock helper, which happily would do the comparison instead (and did until now).

Thank you.

markpeek commented 8 months ago

@ITProKyle could you review and comment/approve based on your previous change?

ITProKyle commented 8 months ago

Sounds reasonable enough to me. Given that no tests were modified, the functionality I implement should still function as intended. @ndparker, mind adding tests to cover this use case as well to ensure it continues to function the same for you?

ndparker commented 8 months ago

Sure, there you go.

I'm a bit unhappy about the duplication of the helper class. But I couldn't find a shared place in the tests directory.

markpeek commented 8 months ago

Thank you for the fix and for adding the tests.

ndparker commented 8 months ago

Thanks for merging. And sorry for not running the formatter before...