Remove Guzzle

[veelasky]

Remove explicitly defined guzzle http client adapter.

And also bump laravel version to support version 8.

[arkaitzgarro]

Hi @veelasky, could you elaborate what’s the reasoning behind removing the Guzzle adapter?

[veelasky]

the underlying HttpClient required by nipwaayoni/elastic-apm-php-agent only states it need to comply with PSR17 and handled by this package php-http/discovery.

Because Guzzle has released version 7, it will be cumbersome if the underlying laravel application uses guzzle version 7, since the adapter for version 6 and 7 is different.

[arkaitzgarro]

Shouldn't be the solution to add php-http/guzzle7-adapter as well, instead of removing adapters? That way the discovery mechanism will pick up the right adapter based on the installed client. @dstepe does this make sense, or will it cause an issue having both adapters?

[dstepe]

I don't think they can both be included. The two Guzzle adapters each require the version of Guzzle they support:

guzzle6-adapter
"guzzlehttp/guzzle": "^6.0"
https://github.com/php-http/guzzle6-adapter/blob/master/composer.json#L21

guzzle7-adapter
"guzzlehttp/guzzle": "^7.0"
https://github.com/php-http/guzzle7-adapter/blob/master/composer.json#L17

That's going to create an immediate dependency conflict.

In general, I advocate that library packages should require the fewest dependencies possible, leaving as much choice to the consumer as they can.

[arkaitzgarro]

I see. @veelasky could you add a suggestion in Composer to install guzzle7-adapter? And I guess we also need to specify in the README that the final user is responsible to install the adapter.

[dstepe]

Laravel 7 introduced an HTTP client (https://laravel.com/docs/7.x/http-client) which is based on Guzzle, so the need to for the user of this package to require Guzzle should be limited to Laravel 6.x unless they want a specific version of Guzzle.

[veelasky]

done. 👍

thanks for the great package, can't wait for the major release.

[arkaitzgarro]

Guzzle references are still present in composer.lock file though. I guess we need to remove guzzlehttp/guzzle, guzzlehttp/promises and guzzlehttp/psr7 right?

[veelasky]

oops sorry, thought it was being ignored by .gitignore

[dstepe]

I believe that most library packages do not include the composer.lock file in source code management. Composer only uses that file when you install dependencies for the package project itself, as in when doing development on the package. It won't have any impact when installing the dependencies of a Laravel application, so any mention of Guzzle in the lock file should not have an impact on the Laravel app install.

That said, running a composer update with the adapter removed does remove the Guzzle package dependency from the lock file, which could then be committed. Of course, that updates a number of other things, but again, that only impacts people who are developing this package.

I would vote to remove composer.lock from the git repo.

[arkaitzgarro]

The main issue removing composer.lock file is that CI runs out of memory when resolving the dependencies. I wouldn't mind removing it, but circleci refuses to work.