[FrameworkBundle] Add integration of http-client component #30419

Ioni14 · 2019-03-01T11:09:58Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

This PR adds the integration of the HttpClient component on FrameworkBundle.
By default, two services are provided, one implementing SFC-HttpClient, and another PSR18:

http_client + its autowiring alias for Symfony\Contracts\HttpClient\HttpClientInterface)
This service is automatically set to the best HTTP client available with the configuration given under the framework.http_client key.
psr18.http_client + its autowiring alias for Psr\Http\Client\ClientInterface). To make it work, one needs to provide autowiring aliases for ResponseFactoryInterface and StreamFactoryInterface, which are provided by the recipe for nyholm/psr7 (but could be overriden by apps when using something else).
one can also configure the default options, and "scoped" clients. For example:

http_client:
    default_options:
        capath: '...'
    clients:
        github_client:
            default_options:
                base_uri: 'https://api.github.com'

This definition create a github_client service implementing SFC-HttpClient and a psr18.github_client one implementing PSR18, +2 corresponding named autowiring aliases: HttpClientInterface $githubClient, and ClientInterface $githubClient.

src/Symfony/Component/HttpClient/Response/CurlResponse.php

src/Symfony/Component/HttpClient/Response/NativeResponse.php

javiereguiluz · 2019-03-01T15:51:25Z

@Ioni14 thanks for your hard work on this feature!

A minor question: could we maintain the global default_options option ... but remove it from the individual HTTP clients? In the example you showed in the description, we'd maintain the first default_options but we'd remove the other two. Don't they look a bit redundant? Thanks.

Ioni14 · 2019-03-01T15:55:26Z

@javiereguiluz thanks ;)

It's probably "YAGNI" but if we want to add other configuration keys for clients in the future it'll be useful to have default_options.

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

nicolas-grekas · 2019-03-11T12:52:24Z

PR and description updated, ready.

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

xabbuh · 2019-03-12T08:45:47Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+                ->setArguments([$options, $clientConfig['max_host_connections']]);
+
+            $container->register('psr18.'.$name, Psr18Client::class)
+                ->setAutowired(true)


Do we really want to use autowiring here? IIRC we don't do that anywhere else with core services.

autowiring allow providing working services once the required aliases are installed
see eg https://github.com/symfony/recipes/blob/master/nyholm/psr7/1.0/config/packages/nyholm_psr7.yaml
This looked the best to achieve loose coupling at the configuration level to me. WDYT?

Maybe related to this, I'm currently getting a fatal error when running debug:container or debug:config with this component enabled in the config:

PHP Fatal error: Method class@anonymous::__toString() must not throw an exception, caught Symfony\Component\DependencyInjection\Exception\AutowiringFailedException: in /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php on line 229 PHP Stack trace: PHP 1. {main}() /Users/weaverryan/Sites/os/docs/http_client_play/bin/console:0 PHP 2. Symfony\Bundle\FrameworkBundle\Console\Application->run() /Users/weaverryan/Sites/os/docs/http_client_play/bin/console:38 PHP 3. Symfony\Bundle\FrameworkBundle\Console\Application->doRun() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:145 PHP 4. Symfony\Bundle\FrameworkBundle\Console\Application->doRun() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:73 PHP 5. Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:269 PHP 6. Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:87 PHP 7. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->run() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:926 PHP 8. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->execute() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Command/Command.php:255 PHP 9. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->getContainerBuilder() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php:117 PHP 10. Symfony\Component\DependencyInjection\ContainerBuilder->compile() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php:213 PHP 11. Symfony\Component\DependencyInjection\Compiler\Compiler->compile() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/ContainerBuilder.php:754 PHP 12. Symfony\Component\DependencyInjection\Compiler\AutowirePass->process() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/Compiler.php:95 PHP 13. Symfony\Component\DependencyInjection\Compiler\AutowirePass->process() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:52 PHP 14. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php:46 PHP 15. Symfony\Component\DependencyInjection\Compiler\AutowirePass->doProcessValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:71 PHP 16. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:99 PHP 17. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php:82 PHP 18. Symfony\Component\DependencyInjection\Compiler\AutowirePass->doProcessValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:71 PHP 19. Symfony\Component\DependencyInjection\Compiler\AutowirePass->autowireCalls() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:122 PHP 20. Symfony\Component\DependencyInjection\Compiler\AutowirePass->autowireMethod() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:164 PHP 21. Symfony\Component\DependencyInjection\Compiler\AutowirePass->Symfony\Component\DependencyInjection\Compiler\{closure}() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:255

This is coming from the psr18.http_client service - not sure what the problem is though!

src/Symfony/Bundle/FrameworkBundle/Resources/config/http_client.xml

src/Symfony/Component/HttpClient/CurlHttpClient.php

src/Symfony/Component/HttpClient/NativeHttpClient.php

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

weaverryan · 2019-03-13T13:47:31Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+                ->setArguments([$options, $clientConfig['max_host_connections']]);
+
+            $container->register('psr18.'.$name, Psr18Client::class)
+                ->setAutowired(true)


Maybe related to this, I'm currently getting a fatal error when running debug:container or debug:config with this component enabled in the config:

PHP Fatal error: Method class@anonymous::__toString() must not throw an exception, caught Symfony\Component\DependencyInjection\Exception\AutowiringFailedException: in /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php on line 229 PHP Stack trace: PHP 1. {main}() /Users/weaverryan/Sites/os/docs/http_client_play/bin/console:0 PHP 2. Symfony\Bundle\FrameworkBundle\Console\Application->run() /Users/weaverryan/Sites/os/docs/http_client_play/bin/console:38 PHP 3. Symfony\Bundle\FrameworkBundle\Console\Application->doRun() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:145 PHP 4. Symfony\Bundle\FrameworkBundle\Console\Application->doRun() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:73 PHP 5. Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:269 PHP 6. Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:87 PHP 7. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->run() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:926 PHP 8. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->execute() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Command/Command.php:255 PHP 9. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->getContainerBuilder() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php:117 PHP 10. Symfony\Component\DependencyInjection\ContainerBuilder->compile() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php:213 PHP 11. Symfony\Component\DependencyInjection\Compiler\Compiler->compile() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/ContainerBuilder.php:754 PHP 12. Symfony\Component\DependencyInjection\Compiler\AutowirePass->process() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/Compiler.php:95 PHP 13. Symfony\Component\DependencyInjection\Compiler\AutowirePass->process() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:52 PHP 14. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php:46 PHP 15. Symfony\Component\DependencyInjection\Compiler\AutowirePass->doProcessValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:71 PHP 16. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:99 PHP 17. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php:82 PHP 18. Symfony\Component\DependencyInjection\Compiler\AutowirePass->doProcessValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:71 PHP 19. Symfony\Component\DependencyInjection\Compiler\AutowirePass->autowireCalls() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:122 PHP 20. Symfony\Component\DependencyInjection\Compiler\AutowirePass->autowireMethod() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:164 PHP 21. Symfony\Component\DependencyInjection\Compiler\AutowirePass->Symfony\Component\DependencyInjection\Compiler\{closure}() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:255

This is coming from the psr18.http_client service - not sure what the problem is though!

weaverryan · 2019-03-13T14:22:29Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

+            ->integerNode('max_host_connections')
+                ->info('The maximum number of connections to a single host.')
+            ->end()
+            ->arrayNode('default_options')


These are the default options for each request that's made. What about default_request_options (it's even documented as Default requests' options in HttpClient phpdoc). And also, should we make these options not automatically be used as the "default options" for all the other HTTP clients?

The problem is that there are 2 concepts conflicting right now. Does "default" mean:
A) default options for the request that can be overridden on each request
OR
B) default options that will be applied to all other HTTP clients

Right now, it's both - that confused me. It's really (A), so maybe we should make it only (A) and clarify the name.

right now, it is default options applied to all other HTTP clients for their default request options...

exactly - the "default" means to things - it's (A) and (B).

Correct, it's A and B. If one wants to configure the default request options for the default client only, one should set them under framework.http_client.clients.http_client.default_options (since http_client is the name of the service that is aliased for HttpClientInterface).
I'm not sure I understand how your suggestion would apply. To me, all is fine :)

I was suggesting that we remove (B), but it's subjective. The question is: is it valuable for these options to cascade onto other clients? Or is it annoying/surprising, and then I am unsetting options on my individual clients, because I don't want them to cascade from my default client?

I'm not convinced that re-using options across clients will be all that common - things like auth, base_uri, headers all seem very client-specific. I could argue that it might be surprising if my auth on my default client starts getting sent to another server via another client (because I didn't realize the options would cascade down to my client).

e.g. cabundle/capath: you want to configure them once for all

Hmm, ok, really fine by me then.

.../FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_full_default_options.xml

weaverryan · 2019-03-13T23:03:43Z

If/when #30547 is merged, the auth option in the config will need to change and the bearer_auth will need to be added.

.../FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_full_default_options.xml

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

weaverryan · 2019-03-17T14:12:19Z

Tests failing on low deps, but other than that - +1 from me!

nicolas-grekas · 2019-03-17T17:04:09Z

Thank you @Ioni14.

nicolas-grekas · 2019-03-17T17:04:10Z

Thank you @nicoweb

…nent (Ioni14, nicoweb) This PR was merged into the 4.3-dev branch. Discussion ---------- [FrameworkBundle] Add integration of http-client component | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This PR adds the integration of the HttpClient component on FrameworkBundle. By default, two services are provided, one implementing SFC-HttpClient, and another PSR18: * `http_client` + its autowiring alias for `Symfony\Contracts\HttpClient\HttpClientInterface`) This service is automatically set to the best HTTP client available with the configuration given under the `framework.http_client` key. * `psr18.http_client` + its autowiring alias for `Psr\Http\Client\ClientInterface`). To make it work, one needs to provide autowiring aliases for `ResponseFactoryInterface` and `StreamFactoryInterface`, which are provided by [the recipe](https://github.com/symfony/recipes-contrib/blob/master/nyholm/psr7/1.0/config/packages/nyholm_psr7.yaml) for `nyholm/psr7` (but could be overriden by apps when using something else). * one can also configure the default options, and "scoped" clients. For example: ```yaml http_client: default_options: capath: '...' clients: github_client: default_options: base_uri: 'https://api.github.com' ``` This definition create a `github_client` service implementing SFC-HttpClient and a `psr18.github_client` one implementing PSR18, +2 corresponding named autowiring aliases: `HttpClientInterface $githubClient`, and `ClientInterface $githubClient`. Commits ------- f2d2cf3 work with attributes for xml config 0023a71 [FrameworkBundle] Add integration of http-client component

carsonbot added Status: Needs Review FrameworkBundle Feature labels Mar 1, 2019

linaori reviewed Mar 1, 2019

View reviewed changes

src/Symfony/Component/HttpClient/Response/CurlResponse.php Outdated Show resolved Hide resolved

src/Symfony/Component/HttpClient/Response/NativeResponse.php Outdated Show resolved Hide resolved

javiereguiluz reviewed Mar 1, 2019

View reviewed changes

sstok reviewed Mar 3, 2019

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved

nicolas-grekas added this to the next milestone Mar 4, 2019

jeremyFreeAgent mentioned this pull request Mar 8, 2019

[HttpClient] Added TraceableHttpClient and WebProfiler panel #30494

Closed

nicolas-grekas mentioned this pull request Mar 9, 2019

[HttpClient] Next steps #30502

Closed

18 tasks

nicolas-grekas approved these changes Mar 11, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 11, 2019

derrabus approved these changes Mar 11, 2019

View reviewed changes

nicolas-grekas mentioned this pull request Mar 11, 2019

Add recipe for http-client symfony/recipes#548

Closed

xabbuh reviewed Mar 12, 2019

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd Outdated Show resolved Hide resolved

weaverryan reviewed Mar 13, 2019

View reviewed changes

stof requested changes Mar 13, 2019

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Mar 13, 2019

nicolas-grekas reviewed Mar 15, 2019

View reviewed changes

.../FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_full_default_options.xml Outdated Show resolved Hide resolved

nicolas-grekas reviewed Mar 15, 2019

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd Show resolved Hide resolved

weaverryan approved these changes Mar 17, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Mar 17, 2019

Thomas Talbot and others added 2 commits March 17, 2019 17:08

[FrameworkBundle] Add integration of http-client component

0023a71

work with attributes for xml config

f2d2cf3

nicolas-grekas merged commit f2d2cf3 into symfony:master Mar 17, 2019

nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

fabpot mentioned this pull request May 9, 2019

Release v4.3.0-BETA1 #31435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle] Add integration of http-client component #30419

[FrameworkBundle] Add integration of http-client component #30419

Ioni14 commented Mar 1, 2019 •

edited by nicolas-grekas

Loading

javiereguiluz commented Mar 1, 2019

Ioni14 commented Mar 1, 2019

nicolas-grekas commented Mar 11, 2019

xabbuh Mar 12, 2019

nicolas-grekas Mar 12, 2019

weaverryan Mar 13, 2019

weaverryan Mar 14, 2019

weaverryan Mar 13, 2019

weaverryan Mar 13, 2019

stof Mar 13, 2019

weaverryan Mar 13, 2019

nicolas-grekas Mar 13, 2019 •

edited

Loading

weaverryan Mar 14, 2019

nicolas-grekas Mar 14, 2019

weaverryan Mar 17, 2019

weaverryan commented Mar 13, 2019

weaverryan commented Mar 17, 2019

nicolas-grekas commented Mar 17, 2019

nicolas-grekas commented Mar 17, 2019

[FrameworkBundle] Add integration of http-client component #30419

[FrameworkBundle] Add integration of http-client component #30419

Conversation

Ioni14 commented Mar 1, 2019 • edited by nicolas-grekas Loading

javiereguiluz commented Mar 1, 2019

Ioni14 commented Mar 1, 2019

nicolas-grekas commented Mar 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas Mar 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weaverryan commented Mar 13, 2019

weaverryan commented Mar 17, 2019

nicolas-grekas commented Mar 17, 2019

nicolas-grekas commented Mar 17, 2019

Ioni14 commented Mar 1, 2019 •

edited by nicolas-grekas

Loading

nicolas-grekas Mar 13, 2019 •

edited

Loading