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

feat: Add retrying to component listing #1190

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

edoardopirovano
Copy link
Contributor

@edoardopirovano edoardopirovano commented Mar 15, 2023

With SAST and SCA getting increasing usage internally we've stated to hit some API rate limits at installation time. We've already put a mitigation place by starting to cache the installed components in the GitHub Actions cache (lacework/code-security-action#44), but ideally we'd like the installation to be a bit more robust to occasional network glitches and/or rate limits being hit. I suggest adding the retrying code in this PR to our call to the component listing end-point. There's probably more places we could have retrying, but this is pretty much the only place I've actually had reports of failures happening, so let's stick to just here for now.

cc. @lacework/codesec-action

Copy link
Contributor

@jeremydubreil jeremydubreil left a comment

Choose a reason for hiding this comment

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

Looks good to me to fix the failures.

lwcomponent/component.go Show resolved Hide resolved
@@ -79,6 +83,13 @@ func LoadState(client *api.Client) (*State, error) {
return nil, errors.New("invalid api client")
}

func backoffStrategy() *backoff.ExponentialBackOff {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this chosen at random?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty much - or at least picked as something that seemed reasonable to me. We can tweak if needed (if we continue to observe issues).

@edoardopirovano edoardopirovano merged commit 317f204 into main Mar 16, 2023
@edoardopirovano edoardopirovano deleted the edoardo/retry-component-listing branch March 16, 2023 16:56
Copy link
Contributor

@afiune afiune left a comment

Choose a reason for hiding this comment

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

Would have loved to see some tests here.

edoardopirovano pushed a commit that referenced this pull request Mar 16, 2023
@edoardopirovano
Copy link
Contributor Author

Would have loved to see some tests here.

A fair point. Added one in #1193.

edoardopirovano added a commit that referenced this pull request Mar 16, 2023
@lacework-releng lacework-releng mentioned this pull request Mar 17, 2023
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.

4 participants