Add auto discovery for GuzzleClient

[aferrandini]

Q	A
Bug?	no
New Feature?	yes

Actual Behavior

If you want to use this Adapter, then you have to instantiate the Guzzle Client and pass through the constructor.

Expected Behavior

Use the php-http/discovery package to auto discover the Guzzle Client and create it.

Possible Solutions

Add the php-http/discovery package as requirement.
Add documentation.

[sagikazarmark]

Hi @aferrandini,

I am a little bit confused, because it seems to me the constructor parameter is optional: https://github.com/php-http/guzzle6-adapter/blob/master/src/Client.php#L28

Also, there is a little bit of confusion on your side too: the discover layer is meant for clients implementing the HTTPlug interfaces or one of the Message Factory interfaces. We are aiming for zero configuration, hence you can instantiate most of our adapters without constructor parameters which allows the discovery layer to find and instantiate them.

Are there any concrete errors you met which led you to creating this issue?

[aferrandini]

Hi @sagikazarmark,

If you want to use the Guzzle Adapter, you have to instantiate Http\Adapter\Guzzle6\Client, but using the discovery package, you only need to call the find method and this will return the Guzzle6 Client instance.

I think the Adapters and Clients should use the discovery package to be able to plug and unplug an adapter or a client without changing anything in the code.

Current usage

$client = new Http\Adapter\Guzzle6\Client();

Expected usage

$client = $httpClient = HttpClientDiscovery::find();

[sagikazarmark]

I am sorry, I still don't get your point. Http\Adapter\Guzzle6\Client implements HTTPlug interfaces (and holds an instance of Guzzle 6 internally). HttpClientDiscovery finds implementations of these interfaces. So as I can see, discovery already works as you described it.

[aferrandini]

Yes, but you have to add the discovery package instead of being a requirement for the adapter package.
Using curl-client you don't need to add the discovery package, because it's included as a requirement.

Curl client composer.json

...
    "require": {
        "php": "^5.5 || ^7.0",
        "ext-curl": "*",
        "php-http/httplug": "^1.0",
        "php-http/message-factory": "^1.0.2",
        "php-http/message": "^1.2",
        "php-http/discovery": "^1.0"
    },
...

Guzzle6 Adapter composer.json

...
    "require": {
        "php": "^5.5 || ^7.0",
        "php-http/httplug": "^1.0",
        "guzzlehttp/guzzle": "^6.0"
    },
...

[sagikazarmark]

Okay, now I see.

The Curl client requires the discovery package because it actually uses it: https://github.com/php-http/curl-client/blob/master/src/Client.php#L80-L81

The discovery package is a convenience layer to allow zero configuration, but the recommended way to create clients is dependency injection.

Also, the discovery package is a dependency of your code, not the clients themselves. As such, you have to install it separately. This might be a little bit inconvenient, but it's the proper way: adding discovery as a dependency to each client is not a good idea. Also, you cannot rely on discovery being available by just installing a client. So in the end you have to add it as a requirement anyway.

[aferrandini]

Okay, I see, so the idea is to remove the discovery and use the client or adapter with DI when possible.
In this case, I'm going to close the issue :)

Thanks so much for the explanation.

[sagikazarmark]

No problem.

Generally Discovery is useful for libraries consuming HTTP clients: API clients for example. Library users then can decide whether they inject a client or fall back to discovery.

See this example:

use Http\Client\HttpClient;
use Http\Discovery\HttpClientDiscovery;

class ApiClient
{
    private $httpClient;

    public function __construct(HttpClient $httpClient = null)
    {
        $this->httpClient = $httpClient ?: HttpClientDiscovery::find();
    }
}

We also try to provide as much framework integrations as possible so that you don't even have to code to configure a client. Symfony bundle is already out, Laravel package is in progress.

[aferrandini]

This example is perfect, it should be on the documentation ;)
I was asking for adding discovery because we're moving an API client with it's CURL integration to HTTPlug :)
Thanks for all the work

[sagikazarmark]

@aferrandini yes, this should definitely be somewhere in the docs. See php-http/documentation#148

If you want to see a better example see this workshop material @dbu and me will present at WebSummerCamp:

https://gitlab.com/dbu/http-client-workshop

[aferrandini]

Thanks ;)