Remove an unreachable code fragment in ec2_security_group #1348
Conversation
ansibullbot
added
community_review
module
There was a problem hiding this comment.
Thanks for taking the time to open this PR.
The description of the PR is a little misleading since the change doesn't remove the code it 'fixes' an accidental bug.
Looking at the integration tests it looks like the expected behaviour (with purge==True) now includes purging the default rules, and looking at the history of the module this has been the behaviour since the creation of this collection, with the behaviour being documented as such since the addition of purge_egress_rules.
Please update this PR to remove the unreachable code. Additionally, please add a changelog fragment: https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to
|
@tremble According to your comment, should I remove the entire if block and keep only the else block(of course after removing the else logic)? |
|
@openrefactory Yes please |
openrefactory
changed the title
|
@tremble Thanks. I have updated the PR and changes. Please check. |
|
recheck (command to CI - ignore me) |
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
This comment was marked as outdated.
This comment was marked as outdated.
|
recheck (command to CI - ignore me) |
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 38s |
This comment was marked as outdated.
This comment was marked as outdated.
|
regate |
|
Build failed (gate pipeline). For information on how to proceed, see https://ansible.softwarefactory-project.io/zuul/buildset/85e0bed80d9641188e96f5bf06f60e7e ❌ ansible-galaxy-importer RETRY_LIMIT in 14s |
|
regate |
|
Build succeeded (gate pipeline). ✔️ ansible-galaxy-importer SUCCESS in 3m 51s |
In file: ec2_security_group.py, method: ensure_present, a logical expression uses the identity operator. A new object is created inside the identity check operation and then used for matching identity. Since this is a distinct, new object, it will not have identity and match with anything else. As a result, the identity check will have a logical short circuit and the program may have unintended behavior.
I suggested that the logical operation should be done properly.