Not sure about the xml conf, advices welcome!

Can you please rebase on 6.3 since this is a bugfix?

Rebased on 6.3!

Edit: Edit: Still got a problem with the way base uris are defined.
Configuration-wise the proposed changes seem to work. But not in the field.

On a fresh install in 6.3:

# config/packages/framework.yamlhttp_client:scoped_clients:test_client:base_uri:                    -'https://jsonplaceholder.typicode.com/'                    -'https://jsonplaceholder.typicode.com/'retry_failed:max_retries:3

// in any controller    #[Route('/', name:'app_client')]publicfunctionindex(HttpClientInterface$testClient):Response    {dump($testClient->request('GET','/posts/1')->toArray());// ...    }

Results in aTypeError: Symfony\Component\HttpClient\ScopingHttpClient::forBaseUri(): Argument #2 ($baseUri) must be of type string, array given since thebase_uri are passed when creating theScopingHttpClient and not theRetryableHttpClient.

Any thought on how to do this? Conditional definition for both seems the most straightforward way, but I'm not sure about that

Made an attempt just as a discussion base. Works as is, but maybe there's a better way to do it or some constraints I haven't tought about.

Hey there!

Sorry for the ping, I know it's been a while, but could I interest you back in a review around here?@nicolas-grekas maybe, or someone else? This was asked in an issue so maybe we should look into it.

In the meantime it needs to be rebased on 6.4 please

Okay there have been obviously a huge mistake on my part here... Fixing asap.

Seems okay with the right destination branch. Sorry for the spam!

Test failures seem unrelated here, but any advice would be welcome!

Hey there!

Bumping this, as it's still a missing feature. Should I create a new PR to get back in the recent ones?

Hi@Tiriel, thanks for your patience.
I think I changed my mind on the topic: this looks like a new feature to me.
To built on this idea, I'm wondering if it wouldn't make more sense to allow a newbase_uris option underretry_failed.
That would make everything simpler I think.

Hi@Tiriel, thanks for your patience. I think I changed my mind on the topic: this looks like a new feature to me.

That's no problem to me, I'll go and change the PR description!

To built on this idea, I'm wondering if it wouldn't make more sense to allow a newbase_uris option underretry_failed. That would make everything simpler I think.

Not sure about that though. I don't know if it's a better idea or not, but I'm not sure about what we should priorize:

• consistency (keep thebase_uri option where it is already defined for the scoped clients)
• intent behind the change (allow to try multiple uris in case of retry)

Both seem to have pros and cons to me, but ultimately I'll get behind the majority advice (or your, if there not much debate ;) )

Also going to rebase and target the latest branch asap

I thinkbase_uris underretry_strategy is more flexible as it allows usingscope on top to control how scoped clients behave

Then how do we deal with thebase_uri option under each scoped client? Unless I miss something, it would be possible to define abase_uri in a scope client and an array ofbase_uris in theretry_strategy.

I see four possibilities:

• adding abase_uri under the scoped client and an array ofbase_uris in the retry strategy is blocked, raising an exception
• adding abase_uri under the scoped client and an array ofbase_uris in the retry strategy isnot blocked, base uri from scoped client is kept
• adding abase_uri under the scoped client and an array ofbase_uris in the retry strategy isnot blocked, base uris from retry strategy are kept
• adding abase_uri under the scoped client and an array ofbase_uris in the retry strategy isnot blocked, base uri from scoped client is merged inside the array

Personnally, I think the first option (blocking) or the last (merging) are the best, with a preference for the blocking.

I would merge them: the base_uri is prepended to base_uris if any.

Hum, actually no: we'd need to make setting bothbase_uri andbase_uris conflict, and we'd then need to compute a regular expression that "|" allbase_uris so that scoped clients accepts them all in its scope (unlessscope is set).

Alternatively, if we keep base_uri as allowing an array, we could make defining aretry_strategy optional (and auto-enable it then).
We also need to allow setting both ascope and an array ofbase_uris for completeness (both options are exclusive at the moment).

WDYT?

Alternatively, if we keep base_uri as allowing an array, we could make defining aretry_strategy optional (and auto-enable it then).

This seems like a better option to me, a bit more friendly to use

We also need to allow setting both ascope and an array ofbase_uris for completeness (both options are exclusive at the moment).

This seems a bit redundant. IIRC,scope makes the configuration active only on uris matching the scope. So if you set an array of base uris and the scope, the scope will simply be active on every call since the base uris matching the scope will be prepended to each call.

In this case, I haven't looked at feasability yet, but maybe we could have a unifiedscope/base_uri option, working differently depending on the actual call you make: when callingrequest, if you pass a full uri with scheme/host, it works as scope, and if you pass a path, it prepends the base uri.

Seems a lot of work though, and not necessarily on the scope of this PR.