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

overcome rate limiting of ListStacks calls #689

Merged
merged 3 commits into from
Apr 2, 2019
Merged

Conversation

errordeveloper
Copy link
Contributor

@errordeveloper errordeveloper commented Apr 1, 2019

Description

This introduces custom retry handler (copied from kops).

I am not sure why DefaultRetryer didn't work, but this adds debug logs, so we will get a better idea of when the retry delay kicks in.

Most importantly it adds a built-in filter for non-deleted stacks that is set by default, and removed an extraneous call to ListStacks.

This fixes #567.

Checklist

  • Code compiles correctly (i.e make build)
  • All unit tests passing (i.e. make test)

@errordeveloper errordeveloper force-pushed the fix-567 branch 2 times, most recently from e7b4c09 to 5361eaa Compare April 1, 2019 15:33
@errordeveloper errordeveloper changed the title Add logging retrier overcome rate limiting of ListStacks calls Apr 1, 2019
@christopherhein
Copy link
Contributor

LGTM! Nice work @errordeveloper

christopherhein
christopherhein previously approved these changes Apr 1, 2019
cdenneen
cdenneen previously approved these changes Apr 1, 2019
Copy link

@cdenneen cdenneen left a comment

Choose a reason for hiding this comment

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

Thanks for the help on this... LGTM!

This often causes a lot of paging, and we only need to list deleted
stacks explicitly in one special case that requires double check if
stack had already been deleted when an error has occurred.
@errordeveloper errordeveloper dismissed stale reviews from cdenneen and christopherhein via 180a5ec April 2, 2019 07:13
@errordeveloper errordeveloper merged commit 8b2e2d6 into master Apr 2, 2019
@errordeveloper errordeveloper deleted the fix-567 branch April 2, 2019 07:21
client.DefaultRetryer
}

var _ request.Retryer = &LoggingRetryer{}
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this variable here? Is it used? Is it supposed to be here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a common way of validating that given struct implements the interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a common way of validating that given struct implements the interface.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors creating and deleting clusters due to CloudFormation throttling
4 participants