Php-graph-sdk: Use HTTPlug instead of our HTTP clients

[yguedidi]

Point 1 in #587

[Nyholm]

It will also make sure we support projects stuck with Guzzle5.
Im happy to provide with a PR if you considering it.

[SammyK]

I wanted to raise a point that I've been hearing from a number of people in the community and have seen in other projects. What is the real benefit of using HTTPlug for our specific use case?

Right now we're going to be pulling in 4 new dependencies with the HTTPlug implementation:

        "psr/http-message": "^1.0",
        "php-http/client-implementation": "^1.0",
        "php-http/httplug": "^1.0",
        "php-http/discovery": "^1.0",

Whereas if we just use Guzzle (the de-facto HTTP client for PHP), we could just do "guzzlehttp/guzzle": "^5.0 || ^6.0" and handle the version differences in the SDK itself. We'd get the benefits of ripping out our custom HTTP client and have only one new dependency.

[Nyholm]

The "real" benefit is that we do not depend on an implementation but an abstraction. Which means that we do not have to bother about Guzzle anymore. As you can see I removed almost 2000 lines of code. And you could even remove more.

The 4 dependencies you are talking about are:

"psr/http-message": "^1.0", // A PSR, only interfaces
"php-http/client-implementation": "^1.0", // A virtual package
"php-http/httplug": "^1.0", // Only interfaces (a soon to be PSR)
"php-http/discovery": "^1.0", // A by the book SOLID package that allows zero config.

The only dependency that is an implementation is the discovery package.

Many libraries has taken this step, including PHPGeocoder, FOSHttpCache, KnpGithub client. Im currently working to create a PSR of HTTPlug. But that will of course take some time.

I described other benefits in my PR: https://github.com/facebook/php-graph-sdk/pull/641#issue-173154393

[SammyK]

As you can see I removed almost 2000 lines of code. And you could even remove more.

Yes, and we'll have the same benefit switching to Guzzle. :)

I described other benefits in my PR...

However some others librairies can be chosen, like Zend/Http or CakePHP one which are already available in their respective framework.

I'm just concerned with the number of dependencies we're having to add. In order to use zend-http I have to pull in the 4 dependencies + the adapter just to use the native framework HTTP client. Whereas if the SDK assumed Guzzle by default - there's one dependency. The tradeoff is that the developer can't choose their own HTTP client.

The question I'm asking is if it's worth it to include 4 extra dependencies on every single project that ever installs the PHP SDK just so that a small subset of developers can use their favorite HTTP client. :)

Less maintenance on this library: HTTPlug offer a large feature set

I don't see how this would be any different than using Guzzle which has the same features.

Better integration with frameworks

A lot of people are already installing a framework-specific adapter to integrate the Facebook PHP SDK (like in Laravel) or are using a third-party package to integrate like The PHP League's Facebook Client or HWIOAuthBundle.

Shared work: Many libraries already implement HTTPlug or are on the way

And a whole heckofa lot more use Guzzle. Just sayin'. :)

[Nyholm]

The question I'm asking is if it's worth it to include 4 extra dependencies on every single project that ever installs the PHP SDK just so that a small subset of developers can use their favorite HTTP client. :)

I will naturally say yes. =) I've been working over a year with HTTPlug because I strongly believe so. It is better software design, it increases flexibility and encourage the community to make HTTP clients better since we (library authors) shares the work. BUT of course, some (as you say) may have opinions that they do not like this. That is fine every library author decides for him/her self. But these opinions is however, not based on software design or best practices.

Dependencies are bad if they are unstable. Large packages that provide an implementation are naturally a greater subject of change. That's why we had Guzzle 4, 5 and 6 releases within two years.

Better integration with frameworks

A lot of people are already installing a framework-specific adapter to integrate the Facebook PHP SDK (like in Laravel) or are using a third-party package to integrate like The PHP League's Facebook Client or HWIOAuthBundle.

Using HTTPlug, Facebook will get a nice Symfony Web Toolbar integration for free. So there is no need of creating a FacebookGraphBundle to show what requests are made and how they were handled.

Shared work: Many libraries already implement HTTPlug or are on the way

And a whole heckofa lot more use Guzzle. Just sayin'. :)

True. I know. Im a maintainer of Guzzle and our plan is to implement HTTPlug as soon as our PSR gets accepted. That will of course be a non-BC breaking change.

[SammyK]

It is better software design, it increases flexibility and encourage the community to make HTTP clients better since we (library authors) shares the work.

On an architectural-astronaut level I agree with you. But this isn't very pragmatic in our case. :) Guzzle has inadvertently become "the abstraction" and I'm OK with using it as such.

So there is no need of creating a FacebookGraphBundle to show what requests are made and how they were handled.

This is a moot point. :)

[...] our plan is to implement HTTPlug as soon as our PSR gets accepted.

I have some pretty big concerns with future-coding something that is so far out and so susceptible to change until it is released.

That will of course be a non-BC breaking change.

Assuming it makes its way through PHP FIG without changes to its current state? :)

Ultimately I'd love to get @yguedidi's input on this and we can definitively go one way or another. :)

[Nyholm]

But this isn't very pragmatic in our case. :)

That is a fair point.

Assuming it makes its way through PHP FIG without changes to its current state? :)

That is a hard requirement from my side. HTTPlug and Guzzle are the two biggest players here. PHP-FIG cannot push though a change that forces a major version bumb for any of these libraries.

I have some pretty big concerns with future-coding something that is so far out and so susceptible to change until it is released.

My point is, this is the direction that the HTTPClient libraries are moving.

[yguedidi]

Choosing Guzzle as a dependency is a business decision.
But the SDK is a library, and by definition, a library aim to be plugged to a business project, it can't be used by its own.
Developers of business projects make business decision, we must not make the decision for them.
So we must be as abstract as possible.

Yes, it's true that Guzzle is the de-facto HTTP client for PHP, and I think it's still the case, but it become the first choice before the rise of HTTPlug, which I hope will become the de-facto abstraction for libraries that need to make HTTP calls.

Like we did by choosing composer as the only install method, or by bumping the PHP version to a supported one, we try to enforce best practices, and I think using HTTPlug will enforce developers to use PSR-7 to make HTTP calls, which is a best practice IMO.
And developers will have the business decision about the implementation in their hands: a Zend project may use Diactoros, an _any framework name here_ project may use Guzzle, a micro-framework project may use Buzz, and so on.

About having many dependencies, it's a problem only when thoses dependencies may conflict in their versions. But with very stable packages like PSR interfaces or HTTPlug interfaces, there is no problem as they will not have as many major versions as Guzzle have haha.
And now, we have composer to handle dependency resolution for us!

Just my 2 cents :)

[yguedidi]

@SammyK @Nyholm any feedback? :)

[Nyholm]

That is a good argument. I agree with you. And I will use your points in the future. =)

[yguedidi]

@Nyholm will you bookmark my comment as "pro-HTTPlug argument"? :smile:

[Nyholm]

I've never considered the business perspective like you do. That is a different approach to the same issue.
I'll bookmark it as "pro-abstraction".

[SammyK]

Sorry for the delay. :)

But with very stable packages like PSR interfaces or HTTPlug interfaces, there is no problem as they will not have as many major versions as Guzzle have

That's the point that I disagree with the most. Guzzle never set out to create very different major versions, it just happened organically. And HTTPlug is still quite new and there is no guarantee that it won't change in the future. In fact I'd be really surprised if there wasn't another major release of HTTPlug in the next year or two. But who knows, maybe HTTPlug is a silver bullet, but I'm very skeptical. :)

In the end, I'll go with @yguedidi as long as no one else on the intertubes has any huge objections. :)

[ollietb]

We've started using this fork in production rather than the current release for the reasons stated above. I hope it gets merged in soon 👍

[yguedidi]

@ollietb thanks for your comment! I'll merge it soon ;)

[Nyholm]

This could now be closed

[kada-zaouak]

merci

[yguedidi]

Closed by #641