8000 [HttpClient] add RetryStrategyInterface by nicolas-grekas · Pull Request #38466 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpClient] add RetryStrategyInterface #38466

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[HttpClient] add RetryStrategyInterface #38466

wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member
Q A
Branch? 5.2
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

Follow up of #38420

Inspired by #38415, this gives more flexibility to the Retry\* logic.

Notably:

  • $retryCount is added to RetryDeciderInterface::shouldRetry()
  • a RetryToken value object is introduced, to provide greater extensibility (adding new methods in the retry logic will be easier via a class vs an interface) and to enable strategies to track a single request across retries (allowing to keep the strategy service stateless when request tracking is desired).
interface RetryStrategyInterface
{
    public function getToken(string $requestMethod, string $requestUrl, array $requestOptions): ?RetryToken;
}

For thoughts, /cc @jderusse

@nicolas-grekas nicolas-grekas added this to the 5.2 milestone Oct 7, 2020
@nicolas-grekas nicolas-grekas deleted the hc-retry-strat branch October 12, 2020 18:40
@nicolas-grekas nicolas-grekas mentioned this pull request Oct 12, 2020
Merged
nicolas-grekas added a commit that referenced this pull request Oct 13, 2020
@nicolas-grekas
feature #38532 [HttpClient] simplify retry mechanism around RetryStra…
f52f090 
…tegyInterface (nicolas-grekas)

This PR was merged into the 5.x branch.

Discussion
----------

[HttpClient] simplify retry mechanism around RetryStrategyInterface

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Replaces #38466

I feel like the mechanism behind RetryableHttpClient is too bloated for no pragmatic reasons.

I propose to merge RetryDeciderInterface and RetryBackoffInterface into one single RetryStrategyInterface.

Having two separate interfaces supports no real-world use case. The only implementations we provide are trivial, and they can be reused by extending the provided GenericRetryStrategy implementation if one really doesn't want to duplicate that.

The methods are also simplified by accepting an AsyncContext as argument. This makes the signatures shorter and allows accessing the "info" of the request (allowing to decide based on the IP of the host, etc).

/cc @jderusse

Commits
-------

544c3e8 [HttpClient] simplify retry mechanism around RetryStrategyInterface
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature HttpClient Status: Needs Review
Projects
None yet
Milestone
5.2
Development

Successfully merging this pull request may close these issues.
2 participants
@nicolas-grekas @carsonbot
0