Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

[Feature] Global configuration to set default retries #759

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sagrawal31
Copy link

@sagrawal31 sagrawal31 commented Mar 7, 2023

WHY are these changes introduced?

For making RestAPI or GraphQL calls, developers often have to set default retries across the application. This is redundant.

WHAT is this pull request doing?

This PR is exposing a global configuration in ConfigParams and is later being used in the HttpClient.

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have used yarn changeset to create a draft changelog entry (do NOT update the CHANGELOG.md file manually)
  • I have added/updated tests for this change
  • I have documented new APIs/updated the documentation for modified APIs (for public APIs)

@sagrawal31 sagrawal31 requested a review from a team as a code owner March 7, 2023 12:08
@sagrawal31
Copy link
Author

Signed the CLA. Not sure how to proceed.

@paulomarg
Copy link
Contributor

Hey, thanks for contributing, and sorry for the delay in responding.

I think this is a good idea, but there are a couple of things that we should consider before merging it:

  • We should add tests to ensure that the global setting is being used by default
  • We should add a test to ensure that the request-specific param overrides the global default to prevent regressions
  • We should probably rename the global setting to defaultHttpRequestTries - we might want to refactor all of the tries params in the future to be retries, but for now they should be consistent

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.

2 participants