Matomo-PHP-API icon indicating copy to clipboard operation
Matomo-PHP-API copied to clipboard

Decouple package from http implementation

Open aschempp opened this issue 2 years ago • 6 comments
trafficstars

I just noticed this package switching from Httpful to Guzzle. It would be great if would not require a http client at all, but let the user choose: http://httplug.io If you are interested in any help or a pull request please let me know 😊

aschempp avatar Jun 12 '23 07:06 aschempp

Yeah but the tests didn't run through, that's why I didn't create a new version yet. But I am working on it right now.

thelfensdrfer avatar Jun 12 '23 09:06 thelfensdrfer

So the new version is released and I still want to make it more independent from the HTTP client library. At the moment you can provide your own GuzzleHttp\Clientclient in the constructor. More to follow in the next release. But it does work with PHP 8.2 without warnings now.

thelfensdrfer avatar Jun 14 '23 09:06 thelfensdrfer

@aschempp I just pushed a new commit implementing psr 18. Do you want to test it? It works with a default guzzlehttp adapter.

Edit: I had to remove the timeout, verifyssl and redirect options though. In psr 18 it is all setup in the beginning. But you can set a new client anytime.

thelfensdrfer avatar Jun 15 '23 11:06 thelfensdrfer

Sorry for being late, and thanks a lot for the effort! I'm not 100% sure it is correct to require php-http/guzzle7-adapter, since that would essentially still install Guzzle by default. As far as I remember (and I only worked with HTTPlug and not PSR-18), you're only supposed to require some of the interfaces and use the client autodiscovery. https://docs.php-http.org/en/latest/httplug/tutorial.html provides an example.

I see that I can possibly replace the client in this library now, but it still installs Guzzle even if another client is available already 😊

aschempp avatar Jul 14 '23 19:07 aschempp

You are correct that httplug works without a default library and it would be more optimal for some people to leave it out. But I think it would be more work for most users, too. I've personally never seen the auto-discovery feature of httplug in the wild. But I also think that removing guzzle as a default would be the best solution for all HTTP libraries long term. Feel free to create a pull request, otherwise I will implement this in the next major release. Until this happens, I will leave the issue open.

thelfensdrfer avatar Jul 15 '23 11:07 thelfensdrfer

Thanks a lot! The project where I currently use this has another side-dependency on Guzzle (https://github.com/alexdebril/rss-atom-bundle). If I find some time to bump the rss-atom-bundle to debril/feed-io v6 I'll also have a look at this 😅

aschempp avatar Jul 21 '23 14:07 aschempp