-
Notifications
You must be signed in to change notification settings - Fork 759
Introducing HTTPlug #475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
We were using ivory before. See #315 as to when it was removed. I personally have nothing against HTTPlug but the docs are currently lacking and, as I have mentioned before, not all PSR-7 implementations include all the interfaces, which makes it harder to use HTTPlug as a one-size-fits-all solution. |
Thanks for the reply @shadowhand. I agree our documentation needs improvement, we are working on it, but there are already examples of usage. As for the PSR-7 argument: I explained in the issue you linked above that HTTPlug does not rely directly on any PSR-7 implementations. We provide message factory interfaces which standardizes construction logic in order to make adapters/clients implementation independent. |
@shadowhand The HTTPlug docs have improved a lot over the last couple of days. Please have a look and tell us if anything is still unclear. |
@shadowhand especially this part should address your concerns: http://php-http.readthedocs.org/en/latest/httplug/library-developers.html#messages |
Is there a specific advantage in switching the HTTP client library that we use? I'm not opposed to changing. I just want to understand the benefits of using something else and why we should make this change. Thanks! |
There is one key feature which I would mention here from a library point of view: Abstraction (and the benefits coming with it) HTTPlug (or probably any client abstraction supporting PSR-7) has a small interface which is the bare minimum for sending HTTP requests. By using an abstraction, your library is not tied to any implementation. Even if you choose to install one with the package, you can easily exchange it without changing the code. From a user point of view: By using an abstraction you open the possibility to use any clients which implements the same API. (Even if through adapters). This is especially useful if there is already a client installed in a project, like zend http client. Otherwise the user would have to install two HTTP clients. (Not to mention if two separate libraries include Guzzle 5 and 6, which would end up in a non-installable package set). Since an HTTP Client is usually encapsulated by the custom logic in a library like this, there is only one or maximum two extension/injection points: the constructor and an optional getter/setter. In this case changing is probably easier and come with the benefit mentioned above. You can also switch to an abstraction "internally" if you want to maintain BC, but in this case you still force a client on the user and only makes sense if you intend to provide full support for the abstraction. |
What would be the BC impact to this library to switch to HTTPlug? Currently, we allow users to inject their own HTTP client, and to retrieve the HTTP client being used, in order to manipulate it. I assume these would be BC breaks? References:
In other places, we use Psr\Http\Message\RequestInterface and Psr\Http\Message\ResponseInterface. Would these change? |
For the long term: yes. Those would be BC breaks (but personally I think that getter/setter should go away. The constructor should be the only injection point). However, for the short term, you can migrate to HTTPlug internally. This means, that you have to install our guzzle6-adapter and inject the Guzzle client into that. After that, you can use that adapter internally (which implements our interface). In the next major version, you can just change the typehint and accept an implementation of our HttpClient. PSR-7 implementations are not necessarily subject of change. You can choose one and use that or you can use our MessageFactory interfaces. That way you can let the user choose the implementation as well. |
The BC breaks are what I was afraid of. This is a major change you're proposing, and I'm not convinced of the value it provides. I do see the value of the abstraction; I don't see the value in breaking BC for it. :-)
I'm open to reviewing a pull request to see how you're thinking this would work.
I think 2.0 is a long way off, unless the rest of the league/oauth2-client provider community sees significant value in fast-tracking this. I am open to that, if everyone else wants to make this change. |
I don't think we've received any requests or complaints about the HTTP client. While I think HTTPlug is a solid library, the cost might be outweighing the benefit in this case. |
I understand this concern. PSR-7 is pretty new (half year now?) and Guzzle 6 comes as the standard PSR-7 client. This is why I suggested the two way migration. Depending on how strict you are about BC (removing typehints and handling specific types, throwing a LogicException when something invalid is injected or preserve the exact same typehints. Technically removing typehints is not a BC break, although auto-wiring might break) HTTPlug can easily be integrated while maintaining BC. I would be happy to provide a PR, even if only for presentation purposes. Contrary what people may think, I don't push immediately dropping any previous code and using HTTPlug. I think it is good to know it's there. We plan to propose a PSR as well, so by the time you get to releasing a new major version, we might have something more widely accepted. |
@sagikazarmark I'll close this issue for now. Feel free to submit a PR. I'd be interested to see what changes you need to make to support it and how they affect BC. We can discuss more on the PR. Thanks! |
I've created a PR for this. #538 |
Hello there,
I am one of the authors of HTTPlug, an HTTP Client abstraction library.
We plan to use this library to implement an OAuth authentication method for our HTTP Mesage library.
I noticed that you rely directly on Guzzle 6 for the HTTP communication. Would you be interested in using an abstraction instead?
We are planning to prepare a two-step migration process to ease migration to HTTPlug without breaking BC. (Details in this issue: php-http/documentation#57)
While we are not stable yet, we encourage people to get known with HTTPlug, as the API is quite stable and we don't plan changing it anymore. FOS HTTP Cache migration is WIP (see this PR FriendsOfSymfony/FOSHttpCache#257)
Let us know if you are interested or need any more information. Our documentation is available here: http://docs.httplug.io
We are also available on slack: http://slack.httplug.io
Thanks,
Mark
The text was updated successfully, but these errors were encountered: