-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpClient] make HttpClient::create()
return an AmpHttpClient
when amphp/http-client
is found but curl is not or too old
#35924
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
Conversation
48a664f
to
00206b5
Compare
HttpClient::create()
return an AmpHttpClient
when amphp/http-client
is found but curl is notHttpClient::create()
return an AmpHttpClient
when amphp/http-client
is found but curl is not or too old
00206b5
to
5b1c137
Compare
5b1c137
to
bc79c8f
Compare
Aweomse, since majority of PaaS outhere have a bad habit of using a really outdated curl version. 🎉 |
@kelunik the test suite of the Messenger component is failing when using
The reproducer is: docker pull feathj/fake-sqs
docker run -d -p 9494:9494 --name sqs feathj/fake-sqs
export MESSENGER_SQS_DSN=sqs://localhost:9494/messages?sslmode=disable
./phpunit src/Symfony/Component/Messenger/ -v --filter Sqs At first, I thought the reason was the The test passes when using the curl or the native client. |
The test passes when I use an |
@nicolas-grekas Do you still have your benchmarking script handy? I was wondering how the Amp client compares to curl, particularly for a single request and many requests to the same host, both HTTP/1.1 and HTTP/2. |
Sure, here it is. <?php
require __DIR__.'/vendor/autoload.php';
$client = new Symfony\Component\HttpClient\AmpHttpClient(['http_version' => 1.1]);
for ($i = 0; $i < 379; ++$i) {
$uri = "https://http2.akamai.com/demo/tile-$i.png";
$responses[] = $client->request('GET', $uri);
}
foreach ($client->stream($responses) as $response => $chunk) {
if ($chunk->isLast()) {
// a $response completed
echo '.';
} else {
// a $response's got network activity or timeout
}
}
echo "\n"; |
0eb451f
to
87c5be9
Compare
…en `amphp/http-client` is found but curl is not or too old
87c5be9
to
7991685
Compare
Green. Status: needs review |
Thank you @nicolas-grekas. |
@nicolas-grekas Thanks for the script again. I see the same results: About the same for HTTP/2, slower for HTTP/1.x. Will have to investigate what's slowing down HTTP/1.x. I was fishing for a reason to have Amp's client be the default client, but I don't really have a compelling reason for that other than installation consistency and portability. 😝 Is there a way to suggest using Amp if the client falls back to using |
We already have an |
Follows #35115
Let's use
amphp/http-client
by default, aftercurl
and beforefopen()
.