-
Notifications
You must be signed in to change notification settings - Fork 395
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
AWS ELB: Return empty list when no load balancer name was found #215
Conversation
1e256fd
to
707825d
Compare
Hi @pjrm thanks for taking the time to raise this PR. I'm not 100% on what behaviour we're want here so I'd like hear from at least @jillr, although as authors of this module @mjschultz and @nand0p could also chime in. |
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 for your contribution here.
In general I think this is the right change but we'd like a couple of extra pieces before we merge this.
- Please add a 'minor_changes' changelog entry: https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to
- We need at least some basic integration testing for the module
- Please update the integration tests in tests/integration/targets/elb_classic_lb/tasks/main.yml to include at least some basic testing for this usecase and a minimal 'standard' use case.
- Then add
elb_classic_lb_info
to tests/integration/targets/elb_classic_lb/aliases - While going the whole hog and adding elb_classic_lb_info calls and assertions after all/most elb_classic_lb calls would be nice, I appreciate that does take time and would be happy with just one for the 'an LB exists' use case and one for the 'no LBs exist' use case.
If you need any help getting these changes added you can find us on IRC (freenode: #ansible-aws) or just ask here.
Hi, you tried to reach out on IRC but unfortunately I was in meetings. Feel free to ask your question on here or in #ansible-aws there's a couple of other folks who may also be able to help. |
707825d
to
709b860
Compare
Hi @tremble, Thank you very much for the feedback. I already provided the requested changes. |
This permission is needed to use the classic elb_classic_info module on elb_classic integration tests (ansible-collections/community.aws#215).
@pjrm this PR contains the following merge commits: Please rebase your branch to remove these commits. |
08797e4
to
568f746
Compare
When trying to describe a LoadBalancer that doesn't exist, the module crash. Instead of that behavior, this commit will return an empty list when no load balancer is found, allowing to deal next tasks by reading the output of the module.
568f746
to
c00e353
Compare
Hi @pjrm thanks for your submission. Sorry it's taken so long to get this merged. |
…ble-collections#215) When trying to describe a LoadBalancer that doesn't exist, the module crash. Instead of that behavior, this commit will return an empty list when no load balancer is found, allowing to deal next tasks by reading the output of the module. Co-authored-by: Pedro Magalhães <[email protected]>
…ble-collections#215) When trying to describe a LoadBalancer that doesn't exist, the module crash. Instead of that behavior, this commit will return an empty list when no load balancer is found, allowing to deal next tasks by reading the output of the module. Co-authored-by: Pedro Magalhães <[email protected]>
…ble-collections#215) When trying to describe a LoadBalancer that doesn't exist, the module crash. Instead of that behavior, this commit will return an empty list when no load balancer is found, allowing to deal next tasks by reading the output of the module. Co-authored-by: Pedro Magalhães <[email protected]>
…ble-collections#215) When trying to describe a LoadBalancer that doesn't exist, the module crash. Instead of that behavior, this commit will return an empty list when no load balancer is found, allowing to deal next tasks by reading the output of the module. Co-authored-by: Pedro Magalhães <[email protected]>
* ec2_vol support for gp3 volume * fix sanity checks * fix reference before assignment * fix integration test * None compare will alway result in True * introduce new parameter 'modify_volume' to keep backwards compatibility * fix output and expand integration test * remove unused key * fix integration test * introduce throughput and fix ci * try latest boto3 * rework boto3 requirements for integration test * add botocode and coverage * boto is also required * add changelog fragment * change version added * change version to majo * change description
When trying to describe a LoadBalancer that doesn't exist, the module crash. Instead of that behavior, this commit will return an empty list when no load balancer is found, allowing to deal next tasks by reading the output of the module.
ISSUE TYPE
COMPONENT NAME
plugins/modules/elb_classic_lb_info.py