php-ovh icon indicating copy to clipboard operation
php-ovh copied to clipboard

Remove dependency on Guzzle

Open shulard opened this issue 8 years ago • 14 comments

We discussed in #34 about the fact that this library is fully coupled to Guzzle. I've worked on that PR to use the php-http/httplug interfaces which allow to decouple from any HTTP client.

In the dev dependencies I still require the Guzzle6 adapter to make all the tests passing.

With that update, it's possible to use the OVH API with all the Httplug adapters (ReactPHP, Socket, Curl, Guzzle5, Guzzle6, Buzz...).

All the projects which requires the OVH API must also require an adapter. It also allow to use the same HTTP client than in the project (just require the adapter).

I've also implemented the HttplugDiscovery plugin which allow to automatically resolve the currently available package through Puli. The only requirement is that the user include the puli composer plugin.

If you have any questions about the implementation, just ask :smile:.

The library usage is the same as before, we can pass an HttpClient instance to the Api client or let the Discovery package resolve the current client.

shulard avatar Apr 04 '16 19:04 shulard

Hi,

Thanks for your contribution. It looks like a great change!

I'm not à PHP developer, but what will happen to code passing a Guzzle instance to the constructor which now expects an adapter ? Is it possible to handle this case to avoid breaking the API ?

yadutaf avatar Apr 04 '16 21:04 yadutaf

Hello! Thanks for the review :smile:.

Insuring non BC break here is a nice idea but it force to include Guzzle as a required dependency. With that we are not taking all the benefits from the decoupling...

I saw that OVH Api change its major version number from Guzzle5 to Guzzle6. Maybe we can use the same here because the HTTP adapter is the hearth of the lib.

If you really want to avoid BC and staying on a 2.x, we can make a decorator to handle the previous Guzzle behaviour and make that decorator the default implementation...

shulard avatar Apr 05 '16 08:04 shulard

Can I assist somehow to make sure this PR get merged? @yadutaf are you 👍 for a change like this?

Nyholm avatar Jul 23 '16 12:07 Nyholm

Any news on that PR ?

shulard avatar Sep 27 '16 08:09 shulard

ping @VincentCasse

yadutaf avatar Sep 27 '16 16:09 yadutaf

Hello guys,

I've rebased on master and fixed the conflicts. Are you open to accept a change like this ?

shulard avatar Dec 13 '16 15:12 shulard

I would also be very interesting in having this merged. I'm trying to integrate this OVH library within a project already using Guzzle5: it's quite problematic. These changes would make it possible to avoid these conflicts.

jas8522 avatar Jan 12 '18 17:01 jas8522

Interested too, with Symfony 4, PSR7, HttPlug is recommended.

Romaxx avatar May 09 '18 10:05 Romaxx

Some files have the conflict because the PR is older than master branch.

These files should be fixed.

peter279k avatar Apr 19 '19 17:04 peter279k

Of course, this PR is 2 years old :smile:. I can update the content, but are the maintainers ok with that kind of update ? If yes I'll work on the rebase asap.

shulard avatar May 13 '19 08:05 shulard

are the maintainers ok with that kind of update ? If yes I'll work on the rebase asap.

:wave: hello :)

we at Nextcloud have a community PR pending that wants to integrate with OVH: https://github.com/nextcloud/twofactor_gateway/pull/316. Thus I would kindly like to know if there is any progress on this matter and/or if anything is missing that could be helped with. What is the status here? :)

Cheers

ChristophWurst avatar Jan 20 '20 16:01 ChristophWurst

Some news about this PR ?

Is there any chance that is merged ?

jdecool avatar Feb 14 '20 06:02 jdecool

I think merging would solve a lot of compatibility issues.

DurandSacha avatar Jul 22 '22 15:07 DurandSacha

Hello !

This is a 6 years old PR, I think that this change will never be merged inside this library.

shulard avatar Jul 25 '22 15:07 shulard