-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
.../input/entityanalytics/provider/okta: Handle 429s, concurrent limits #42094
.../input/entityanalytics/provider/okta: Handle 429s, concurrent limits #42094
Conversation
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
97a7acf
to
35940af
Compare
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.
nits only
return nil, nil, fmt.Errorf("maximum retries (%d) finished without success", maxRetries) | ||
} | ||
if retryCount > 0 { | ||
log.Warnf("retrying... (%d/%d)", retryCount, maxRetries) |
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.
Prefer log._w
over log._f
.
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.
Done
} | ||
ctx := context.Background() | ||
log := logp.L() | ||
e := r.endpoint(endpoint) |
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.
Linter is correct here, this is an ineffassign.
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.
The line is actually unnecessary. Removed.
"github.com/elastic/elastic-agent-libs/logp" | ||
"golang.org/x/time/rate" |
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.
Needs goimports or manual separation.
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.
This was less automatic than I expected. Fixed.
650a97c
to
6597840
Compare
6597840
to
1eb9a34
Compare
Proposed commit message
Discussion
Hiding whitespace changes will greatly improve the readability of the diff for
x-pack/filebeat/input/entityanalytics/provider/okta/internal/okta/okta.go
. It's mostly indentation changes.For the concurrent rate limit, the previous behavior was to reset to a rate of zero, which would cause an error because the next wait would exceed the maximum wait time.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Related issues