Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[HttpClient] Parameterize list of retryable methods#38426

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:5.xfromjderusse:retry-method
Oct 20, 2020

Conversation

@jderusse
Copy link
Member

@jderussejderusse commentedOct 6, 2020•
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
Tickets/
LicenseMIT
Doc PRTODO

Retrying non-idempotent methods is not always acceptable for user. This PR adds an easy way to configure this behavior.

TheRetryDeciderInterface::shouldRetry() now take the exception in parameter, in order to let decider not retrying the request when the methods should never by retried.

With#38420, this code would belongs to the RetryStrategy implementation, and would return anNeverRetryDecider when method is not allowed.

@jderusse
Copy link
MemberAuthor

I wonder if the implementation/configuration shouldn't be more complicated:

  • 423, 425, 429, 503 => always
  • 500, 502, 504, 507, 510 => idempotent only

Maybe:
ChainDecider

  • StatusCodeDecider([423, 425, 429, 503])
  • IdempotentStatusCodeDecider([500, 502, 504, 507, 510], [GET, HEAD, PUT, DELETE, TRACE, OPTIONS, CONNECT]

@jderussejderusseforce-pushed theretry-method branch 3 times, most recently from68a37b1 to070c1beCompareOctober 6, 2020 15:16
@nicolas-grekasnicolas-grekas added this to the5.2 milestoneOct 6, 2020
@jderussejderusse changed the base branch frommaster to5.xOctober 6, 2020 21:47
@jderussejderusseforce-pushed theretry-method branch 2 times, most recently from2d35c26 toe40e360CompareOctober 6, 2020 21:48
@jderusse
Copy link
MemberAuthor

failling test are related to Could not parse version constraint >=5.x: Invalid version string "5.x" And related to async-aws that should be fixed byasync-aws/aws#802

@jderussejderusseforce-pushed theretry-method branch 2 times, most recently fromf6270bb to73db919CompareOctober 7, 2020 10:59
Copy link
Member

@NyholmNyholm left a comment

Choose a reason for hiding this comment

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

Thank you. I like this. I added a bunch of questions for things I dont understand.

@jderussejderusseforce-pushed theretry-method branch 5 times, most recently from3f9fb7a toa7f7ec5CompareOctober 8, 2020 11:11
@nicolas-grekas
Copy link
Member

Let's settle on#38532 before merging (and rebasing) this one.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

for exceptions, we could use the status code0

@jderussejderusseforce-pushed theretry-method branch 2 times, most recently from2c1ee99 to83d8d10CompareOctober 16, 2020 16:53
@fabpot
Copy link
Member

Thank you@jderusse.

@fabpotfabpot merged commit185e9ea intosymfony:5.xOct 20, 2020
@jderussejderusse deleted the retry-method branchOctober 25, 2020 19:49
@fabpotfabpot mentioned this pull requestOct 28, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@NyholmNyholmNyholm left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

5 participants

@jderusse@nicolas-grekas@fabpot@Nyholm@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp