search

thephpleague / omnipay-common

Core components for the Omnipay PHP payment processing library
MIT License
330 stars 244 forks source link

[3.0] Add HTTP Factories for Guzzle #125

Closed barryvdh closed 8 years ago

barryvdh commented 8 years ago

I think this cleans up the factories pretty well, as we don't need to inject a lot of dependancies everywhere. Note that we do require on Guzzle/psr7 for this, but only internally, so we can upgrade to other PSR-7 factories when required.

harikt commented 8 years ago

The HTTP Factory PSR seems to be stalled, so I created my own factory with similar methods.

Do you know there exists https://github.com/http-interop ? It has different factories implementation also.

barryvdh commented 8 years ago

Yes, that isn't stable or widely adopted yet.

harikt commented 8 years ago

@barryvdh you can point to a release https://github.com/http-interop/http-factory/releases .

Not widely adopted is for it is WIP . But you should consider discussing with @shadowhand , @nyamsprod for they also part of League and they have some of the implementations based on it. ( https://github.com/bakame-php/http-client ) .

barryvdh commented 8 years ago

@barryvdh you can point to a release https://github.com/http-interop/http-factory/releases . Yes, but that's still a 0.1 release, and PSR-17 is not yet accepted.

There also is https://github.com/php-http/message-factory from @sagikazarmark and other, which at least is stable, but also not widely adopted yet.

I could see myself adding a php-http Client, besides the default Guzzle one. But not sure how relevant a Http Factory abstraction is. What would be the main benefit, avoiding duplicate PSR-7 libraries? Perhaps also avoid package lock-in, although we don't expose the internals, so should be able to upgrade without breaking BC..

harikt commented 8 years ago

sure :+1: . Thanks for the information.

barryvdh commented 8 years ago

I did some more reading, and I do kind of like both projects, but the problem is that:

Due to the problems of requiring external interfaces now (Guzzle 3 Http client), I would really like to be sure the interface is here to stay for a good amount of time and otherwise use our own interface/class. And we really can't keep waiting for another half year.

nyamsprod commented 8 years ago

@barryvdh exactly, the http-interop for the factory is nearly complete (don't be distracted by the 0.1.0 tag) so If I were to consider one of them I would go with the http-interop interfaces but @shadowand may better answer this question. What is sure is that whatever changes may happen in the factories adding a "simple" wrapper around the current interface will solve the issue.

sagikazarmark commented 8 years ago

I agree with @nyamsprod given PSR-17 is finished within a certain time.

php-http/message-factories: seems temporary solution until PSR-17 is released

That is not correct, we started the project way earlier than PSR-17 was proposed. As such, many of our projects rely on it, so we are not going to drop the support for it. Also, I am planning to provide bridges in both direction, so new software can use the old interfaces, and an old software can use the new implementations.

barryvdh commented 8 years ago

That is not correct, we started the project way earlier than PSR-17 was proposed. As such, many of our projects rely on it, so we are not going to drop the support for it.

Okay apologies, I meant that it seems that php-http/message-factories is already planned on being deprecated based on https://github.com/php-http/message-factory/issues/32#issuecomment-240646097

3 Once this proposal reaches standard phase, deprecate this factory package and recommend PSR-17

So while it might still receive support, it would be better to follow the 'official' one :)

sagikazarmark commented 8 years ago

Okay apologies

No problem, just wanted to provide you correct information.