Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Cleanup ec2_elb* #586

Merged
merged 8 commits into from
Jul 9, 2021
Merged

Cleanup ec2_elb* #586

merged 8 commits into from
Jul 9, 2021

Conversation

tremble
Copy link
Contributor

@tremble tremble commented May 28, 2021 •
edited
Loading

SUMMARY

This effectively reverts ansible/ansible#30532

@wimnat originally tried to keep ec2_elb, ec2_elb_lb and their fact/info partners separate from elb_instance / elb_classic_lb so that when a boto3 migration was performed the API/return values could be changed. We no longer have a 'preview' flag on the interface, so this is just causing duplication.

  • ec2_elb_info now has a boto3 based equivalent elb_classic_lb_info deprecate it so we can drop the old boto based module
  • elb_classic_lb was never rewritten, point it at amazon.aws.ec2_elb_lb so we only try and do the boto3 migration in one place
  • elb_instance was never rewritten, point ec2_elb at community.aws.elb_instance so we only try and do the boto3 migration in one place

Before we release 2.0.0 let's get this back to something vaguely consistent.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_elb
ec2_elb_lb
ec2_elb_info
elb_classic_lb
elb_classic_lb_info

ADDITIONAL INFORMATION

@ansibullbot
Copy link

cc @jarv @jillr @jsdalton @mjschultz @nand0p @s-hertel
click here for bot help

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module needs_triage plugins plugin (any type) labels May 28, 2021
@tremble tremble requested a review from jillr May 28, 2021 11:12
@jillr jillr self-assigned this Jun 1, 2021
@jillr
Copy link
Collaborator

jillr commented Jun 1, 2021

This issue helped me understand the history so leaving it here for anyone else who comes across this PR:
ansible/ansible#30494

Thanks for untangling this @tremble

jillr
jillr reviewed Jun 1, 2021
View reviewed changes
Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a changelog explaining that there was code duplication and that's why we already removed the file even though the redirect says it won't go away until 3.0. Folks might go looking for the module on-disk if they're debugging something.

@tremble tremble force-pushed the cleanup/elb branch 3 times, most recently from e8a23ee to 82bb850 Compare June 8, 2021 12:53
@jillr
Copy link
Collaborator

jillr commented Jul 7, 2021

@tremble I'm +1 to gate this once the merge conflicts are resolved, thanks!

@tremble tremble force-pushed the cleanup/elb branch 2 times, most recently from 1b77ac6 to cd92b12 Compare July 8, 2021 15:25
@ansibullbot
Copy link

cc @markuman
click here for bot help

markuman
markuman approved these changes Jul 9, 2021
View reviewed changes
Copy link
Member

@markuman markuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tremble tremble added the gate label Jul 9, 2021
@ansible-zuul ansible-zuul bot merged commit aaa1327 into ansible-collections:main Jul 9, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 15, 2021
@ansible-zuul
[promoted]Merge pull request ansible-collections#586 from tremble/cle…
f29f90c 
…anup/elb

Cleanup ec2_elb*

SUMMARY
This effectively reverts ansible/ansible#30532
@wimnat originally tried to keep ec2_elb, ec2_elb_lb and their fact/info partners separate from elb_instance / elb_classic_lb so that when a boto3 migration was performed the API/return values could be changed.  We no longer have a 'preview' flag on the interface, so this is just causing duplication.

ec2_elb_info now has a boto3 based equivalent elb_classic_lb_info deprecate it so we can drop the old boto based module
elb_classic_lb was never rewritten, point it at amazon.aws.ec2_elb_lb so we only try and do the boto3 migration in one place
elb_instance was never rewritten, point ec2_elb at community.aws.elb_instance so we only try and do the boto3 migration in one place

Before we release 2.0.0 let's get this back to something vaguely consistent.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_elb
ec2_elb_lb
ec2_elb_info
elb_classic_lb
elb_classic_lb_info
ADDITIONAL INFORMATION

Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <git@osuv.de>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@aaa1327
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 16, 2021
@ansible-zuul
[promoted]Merge pull request ansible-collections#586 from tremble/cle…
d97077f 
…anup/elb

Cleanup ec2_elb*

SUMMARY
This effectively reverts ansible/ansible#30532
@wimnat originally tried to keep ec2_elb, ec2_elb_lb and their fact/info partners separate from elb_instance / elb_classic_lb so that when a boto3 migration was performed the API/return values could be changed.  We no longer have a 'preview' flag on the interface, so this is just causing duplication.

ec2_elb_info now has a boto3 based equivalent elb_classic_lb_info deprecate it so we can drop the old boto based module
elb_classic_lb was never rewritten, point it at amazon.aws.ec2_elb_lb so we only try and do the boto3 migration in one place
elb_instance was never rewritten, point ec2_elb at community.aws.elb_instance so we only try and do the boto3 migration in one place

Before we release 2.0.0 let's get this back to something vaguely consistent.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_elb
ec2_elb_lb
ec2_elb_info
elb_classic_lb
elb_classic_lb_info
ADDITIONAL INFORMATION

Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <git@osuv.de>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@aaa1327
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 17, 2021
@ansible-zuul
[promoted]Merge pull request ansible-collections#586 from tremble/cle…
75d97cd 
…anup/elb

Cleanup ec2_elb*

SUMMARY
This effectively reverts ansible/ansible#30532
@wimnat originally tried to keep ec2_elb, ec2_elb_lb and their fact/info partners separate from elb_instance / elb_classic_lb so that when a boto3 migration was performed the API/return values could be changed.  We no longer have a 'preview' flag on the interface, so this is just causing duplication.

ec2_elb_info now has a boto3 based equivalent elb_classic_lb_info deprecate it so we can drop the old boto based module
elb_classic_lb was never rewritten, point it at amazon.aws.ec2_elb_lb so we only try and do the boto3 migration in one place
elb_instance was never rewritten, point ec2_elb at community.aws.elb_instance so we only try and do the boto3 migration in one place

Before we release 2.0.0 let's get this back to something vaguely consistent.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_elb
ec2_elb_lb
ec2_elb_info
elb_classic_lb
elb_classic_lb_info
ADDITIONAL INFORMATION

Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <git@osuv.de>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@b946932
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@ansible-zuul
Merge pull request ansible-collections#586 from tremble/cleanup/elb
099b13f 
Cleanup ec2_elb*

SUMMARY
This effectively reverts ansible/ansible#30532
@wimnat originally tried to keep ec2_elb, ec2_elb_lb and their fact/info partners separate from elb_instance / elb_classic_lb so that when a boto3 migration was performed the API/return values could be changed.  We no longer have a 'preview' flag on the interface, so this is just causing duplication.

ec2_elb_info now has a boto3 based equivalent elb_classic_lb_info deprecate it so we can drop the old boto based module
elb_classic_lb was never rewritten, point it at amazon.aws.ec2_elb_lb so we only try and do the boto3 migration in one place
elb_instance was never rewritten, point ec2_elb at community.aws.elb_instance so we only try and do the boto3 migration in one place

Before we release 2.0.0 let's get this back to something vaguely consistent.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_elb
ec2_elb_lb
ec2_elb_info
elb_classic_lb
elb_classic_lb_info
ADDITIONAL INFORMATION

Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <git@osuv.de>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@ansible-zuul
Merge pull request ansible-collections#586 from tremble/cleanup/elb
7c5df5d 
Cleanup ec2_elb*

SUMMARY
This effectively reverts ansible/ansible#30532
@wimnat originally tried to keep ec2_elb, ec2_elb_lb and their fact/info partners separate from elb_instance / elb_classic_lb so that when a boto3 migration was performed the API/return values could be changed.  We no longer have a 'preview' flag on the interface, so this is just causing duplication.

ec2_elb_info now has a boto3 based equivalent elb_classic_lb_info deprecate it so we can drop the old boto based module
elb_classic_lb was never rewritten, point it at amazon.aws.ec2_elb_lb so we only try and do the boto3 migration in one place
elb_instance was never rewritten, point ec2_elb at community.aws.elb_instance so we only try and do the boto3 migration in one place

Before we release 2.0.0 let's get this back to something vaguely consistent.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_elb
ec2_elb_lb
ec2_elb_info
elb_classic_lb
elb_classic_lb_info
ADDITIONAL INFORMATION

Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <git@osuv.de>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@ansible-zuul
Merge pull request ansible-collections#586 from tremble/cleanup/elb
b946932 
Cleanup ec2_elb*

SUMMARY
This effectively reverts ansible/ansible#30532
@wimnat originally tried to keep ec2_elb, ec2_elb_lb and their fact/info partners separate from elb_instance / elb_classic_lb so that when a boto3 migration was performed the API/return values could be changed.  We no longer have a 'preview' flag on the interface, so this is just causing duplication.

ec2_elb_info now has a boto3 based equivalent elb_classic_lb_info deprecate it so we can drop the old boto based module
elb_classic_lb was never rewritten, point it at amazon.aws.ec2_elb_lb so we only try and do the boto3 migration in one place
elb_instance was never rewritten, point ec2_elb at community.aws.elb_instance so we only try and do the boto3 migration in one place

Before we release 2.0.0 let's get this back to something vaguely consistent.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_elb
ec2_elb_lb
ec2_elb_info
elb_classic_lb
elb_classic_lb_info
ADDITIONAL INFORMATION

Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <git@osuv.de>
@tremble tremble deleted the cleanup/elb branch November 26, 2021 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markuman markuman markuman approved these changes

@jillr jillr jillr approved these changes

Assignees

@jillr jillr

Labels
community_review feature This issue/PR relates to a feature request module module plugins plugin (any type) tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tremble @ansibullbot @jillr @markuman