-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fixed tf tests: TestAccRoute53HealthCheck #5241
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5241 +/- ##
==========================================
- Coverage 95.87% 95.82% -0.06%
==========================================
Files 688 688
Lines 68426 68484 +58
==========================================
+ Hits 65603 65622 +19
- Misses 2823 2862 +39
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
moto/route53/models.py
Outdated
self.caller_reference = caller_reference | ||
|
||
def add_children(self, children): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name set_children
would be more accurate, as it doesn't add anything to the existing list of children
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Updated
moto/route53/models.py
Outdated
self.children = [children] | ||
else: | ||
self.children = None | ||
return self.children |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return value is not used anywhere, as far as I can tell, so we can remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. Missed it. Thanks for pointing out.
moto/route53/models.py
Outdated
elif children and isinstance(children, str): | ||
self.children = [children] | ||
else: | ||
self.children = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this desired behaviour?
When calling update_healthcheck
without the children
-argument, this removes any existing children. It seems more intuitive to keep the existing children, if the user doesn't explicitly say they want to change/update them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. if calling update_healthcheck
without --child-health-checks
argument - its not removing any children. Fixed this.
But it will overwrite all children if --child-health-checks
argument is present with valid children.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @macnev2013!
This is now part of moto >= 3.1.14.dev19 |
TF Tests
Fixed
Added