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.

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

Attempt to improve vpc_subnet / vpc_net stability #270

Merged
merged 6 commits into from
Mar 8, 2021

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Feb 25, 2021

SUMMARY

Over in community.aws we're seeing repeated failures due to API rate limits with the subnet/net changes (mostly waiter related)
Switch to a custom waiter that knows how to cope with the API Rate limits.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_vpc_subnet
ec2_vpc_net

ADDITIONAL INFORMATION

Will push changelog once tests complete

@ansibullbot
Copy link

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module module_utils module_utils needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR needs_triage plugins plugin (any type) labels Feb 25, 2021
@tremble tremble removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Feb 25, 2021
Copy link
Collaborator

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

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

Thank you @tremble for asking me to review.

plugins/module_utils/waiters.py Show resolved Hide resolved
plugins/module_utils/waiters.py Show resolved Hide resolved
plugins/module_utils/waiters.py Show resolved Hide resolved
plugins/modules/ec2_vpc_net.py Show resolved Hide resolved
plugins/modules/ec2_vpc_net.py Show resolved Hide resolved
plugins/modules/ec2_vpc_net.py Show resolved Hide resolved
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Mar 4, 2021
@tremble
Copy link
Contributor Author

tremble commented Mar 6, 2021

@alinabuzachis For what it's worth I did dig into what it would take to tune the botocore waiters

In theory we could do so by using botocore.client.BaseClient._get_waiter_config() (client._get_waiter_config) however, as an _ method that leaves us reliant on a non-public interface, which I don't think would be a good idea.

This leaves is with copying the waiter configs we care about into waiters.py as the only sane way to inject retrying on API rate limits.

@alinabuzachis
Copy link
Collaborator

@alinabuzachis For what it's worth I did dig into what it would take to tune the botocore waiters

In theory we could do so by using botocore.client.BaseClient._get_waiter_config() (client._get_waiter_config) however, as an _ method that leaves us reliant on a non-public interface, which I don't think would be a good idea.

This leaves is with copying the waiter configs we care about into waiters.py as the only sane way to inject retrying on API rate limits.

@tremble Thank you for explaining. It makes sense to me. I am going to add a waiter even for NatGatewayAvailable since I used the built-in one. https://github.com/ansible-collections/community.aws/pull/445/files#diff-847118175f7fbd015daf612a3319157ca22874f706c01f26348f4f86f67384d9R706-R709

@ansibullbot ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Mar 8, 2021
@tremble
Copy link
Contributor Author

tremble commented Mar 8, 2021

Change approved and Shippable tests passing. Merging

@tremble tremble merged commit c89b3fc into ansible-collections:main Mar 8, 2021
@tremble tremble deleted the stability/vpc branch May 28, 2021 07:08
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…ble-collections#270)

* Add additional storage classes to S3 lifecycle transition list.
* Add minor changes changelog for S3 lifecycle transition list change.
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…ble-collections#270)

* Add additional storage classes to S3 lifecycle transition list.
* Add minor changes changelog for S3 lifecycle transition list change.
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
…ble-collections#270)

* Add additional storage classes to S3 lifecycle transition list.
* Add minor changes changelog for S3 lifecycle transition list change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request module_utils module_utils module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants