Correct non-existent exception in ec2_asg #2527
Conversation
|
Hi @shawnsi -- thanks for submitting this PR. @garethr -- Please review this PR to make sure it adheres to the following guidelines: http://docs.ansible.com/developing_modules.html#module-checklist If it passes these guidelines, and if you believe it’s a good PR otherwise, please add a comment with "shipit" in the text, and we will flag it for inclusion. |
|
Thanks @shawnsi for this PR. Unfortunately, it is not mergeable in its current state due to merge conflicts. Please rebase your PR. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. |
|
ready_for_review |
shawnsi
force-pushed
the
fix2526
branch
2 times, most recently
from
68ba008
to
0cfa3d3
Compare
|
Thanks @shawnsi. @garethr please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit' or 'needs_revision' as appropriate. |
1 similar comment
|
Thanks @shawnsi. @garethr please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit' or 'needs_revision' as appropriate. |
|
@garethr This change is still pending your review; do you have time to take a look and comment? Please comment with text 'shipit' or 'needs_revision' as appropriate. |
|
@robynbergeron @garethr can someone please merge this? Functionality is completely broken for me and I'd rather not run production deployments off someone's forked branch. |
|
@garethr still waiting on your review. Please comment with text 'shipit' or 'needs_revision' as appropriate. If we don't hear from you within 14 days, we will start to look for additional maintainers for this module. |
|
Moving into review by the @ansible/core team. @shawnsi apologies for the delay. |
|
I've updated the exception handling per this comment in the issue. |
|
@gregdek Is there any progress with this pull request? |
|
@gregdek Are you still looking to identify a maintainer? This change is pretty simple and would be very helpful if merged. |
|
I'm gonna put this one into shipit and see what happens. :) |
| @@ -333,8 +333,12 @@ def elb_healthy(asg_connection, elb_connection, module, group_name): | |||
| # but has not yet show up in the ELB | |||
| try: | |||
| lb_instances = elb_connection.describe_instance_health(lb, instances=instances) | |||
| except boto.exception.InvalidInstance: | |||
| pass | |||
| except boto.exception.BotoServerError, e: | |||
There was a problem hiding this comment.
Just to keep things happy on the python3 front, this should be except boto.exception.BotoServerError as e:
|
@shawnsi One minor nit on this one- ping me when it's fixed and I'll merge. Thanks! |
Fail on unhandled exception rather than raise
|
@nitzmahone Its fixed. Thanks for the prompt review. |
|
Noice- thanks! |
Fail on unhandled exception in ec2_asg rather than raise
Fail on unhandled exception in ec2_asg rather than raise
Fixes #2526