azure_rm_securitygroup - idempotent when args are lists #507
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tfmark
force-pushed
the
securitygroup-stringy-ports
branch
from
09f2949 to
ba81fb2
Compare
tfmark
force-pushed
the
securitygroup-stringy-ports
branch
from
ba81fb2 to
d962d08
Compare
Fred-sun
reviewed
Apr 28, 2021
| @@ -475,6 +475,8 @@ def compare_rules_change(old_list, new_list, purge_list): | |||
|
|
|||
|
|
|||
| def compare_rules(old_rule, rule): | |||
| def compare_list_rule(old_rule, rule, key): | |||
There was a problem hiding this comment.
It would be even better if the compare_list_rule function were defined outside compare_rules. Thank you!
There was a problem hiding this comment.
I was following the similar example here:
. I can change, but guess it should be a static method as it doesn't need anything from the class (unless you're ignoring linter warnings about static methods?)There was a problem hiding this comment.
yes， I will confirm with developer! Thank you very much!
Fred-sun
added
medium_priority
haiyuazhang
force-pushed
the
dev
branch
2 times, most recently
from
2281f46 to
8dfc8ed
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Attempting to fix #504
ISSUE TYPE
COMPONENT NAME
azure_rm_securitygroup
ADDITIONAL INFORMATION
This commit hopefully has a broken unit test. second commit may fix it. Haven't figured out how to test locally 😢
edit Well my test case that should have failed, passed. but yeh. This change fixes the behaviour locally. I don't know why the test case didn't highlight the problem.
edit2 Are the tests even ran? I had a typo in the unit test and it still passed the github action...