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 retry functionality to the splunk_rest_client #392

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

sgoral-splunk
Copy link
Contributor

@sgoral-splunk sgoral-splunk commented Sep 30, 2024

Issue number:ADDON-75327

Summary

Added functionality to retries requests.

Changes

Added "max_retries" to the "HTTPAdapter" and set it to 5.

User experience

No impact / potential reliability improvement

Checklist

If your change doesn't seem to apply, please leave them unchecked.

@sgoral-splunk sgoral-splunk marked this pull request as ready for review September 30, 2024 09:59
@sgoral-splunk sgoral-splunk requested a review from a team as a code owner September 30, 2024 09:59
@sgoral-splunk sgoral-splunk merged commit 42e0093 into develop Sep 30, 2024
15 of 16 checks passed
@sgoral-splunk sgoral-splunk deleted the fix/add_retry_to_requests branch September 30, 2024 10:02
@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 2024
@srv-rr-github-token
Copy link
Contributor

🎉 This PR is included in version 5.2.0-beta.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@srv-rr-github-token
Copy link
Contributor

🎉 This PR is included in version 5.3.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@srv-rr-github-token
Copy link
Contributor

🎉 This PR is included in version 5.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Comment on lines +103 to +109
retries = Retry(
total=MAX_REQUEST_RETRIES,
backoff_factor=0.3,
status_forcelist=[500, 502, 503, 504],
allowed_methods=["GET", "POST", "PUT", "DELETE"],
raise_on_status=False,
)

Choose a reason for hiding this comment

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

Should get config from context and fill with defaults when not present

Choose a reason for hiding this comment

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

You're right, that's the plan. We're going to refactor this code soon.

Copy link
Member

Choose a reason for hiding this comment

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

@dkvashninsplunk this was done as a temporary solution to unblock other team

we want to get rid of requests library at all for solnlib and splunktaucclib so this code might potentially be deleted once we have a solution

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

Successfully merging this pull request may close these issues.

5 participants