Guzzle 7.0 ready #72
Guzzle 7.0 ready #72
Conversation
|
One point of view is what you say but ppl will not be able to update to a new version of guzzle that might be faster with new features and not buggy... Then they will drop guzzle and use the curl client of http lug.. |
I would like to disagree here. HTTPlug is used mainly by libraries/bundles, not by the end projects. As a project's developer, I want to upgrade Guzzle to 7.x, but if HTTPlug (required by some libs that I use) doesn't support this version, I will be either stuck with Guzzle 6.x or forced to use a different adapter (curl). Both options don't look pretty to me. The goal of this PR is to allow the end developer to upgrade to Guzzle 7.x and still use libs that require HTTPlug. |
|
Right, conflicting dependencies might be annoying to people. |
|
I think we should tweak the configuration for Travis CI to have dedicated jobs for both support major Guzzle versions to ensure that we detect failures for each of them as soon as possible. |
|
Sounds good! I'd do it in a separate PR together with moving to GitHub actions. |
|
just for the naming, i think we should create guzzle7-adapter rather than allow guzzle6-adapter to support guzzle 7. then the readme of the guzzle7 adapter can also explain the use case for this, and say that if all you need is a psr-18 http client, just use guzzle directly without an additional adapter. |
|
Argggg.. I forgot that the name is |
|
maybe we could even call the new repo guzzle-async-adapter and only provide the async client adapter. (as having a sync adapter is pointless and leads people to write bad code). i would prefer to drop the version name in the async adapter, hoping that guzzle 8 will not BC break in a hard way, but even if it does, we can do adapter version 2 that only works with guzzle 8, while version 1 works with guzzle 7. |
|
Hello, Sentry uses HTTPlug with the guzzle6-adapter by default. Is there any way to use HTTPlug without an adapter? I guess not, because sentry uses php-http/async-client-implementation as an dependency. |
|
Is there any work being done on this elsewhere, or has it stalled awaiting a decision on repository naming? I'd like to help get it moving again. |
|
i am not aware of anyone working on this currently. shall we go with guzzle-async-adapter? i can create the repository if somebody plans to work on it. |
|
Are there any updates on this? I'm also running into an issue where Package A requires a The acceptance of this PR would solve that problem. |
|
guzzle 7 natively provides it would be super weird to have the guzzle6-adapter support guzzle 7. plus it would be unnecessary overhead to provide an adapter for guzzle 7 synchronous clients when that client itself already implements psr 18. the only thing that would make sense imho would be a php-http adapter for guzzle 7 asynchronous requests, because those are not defined by any PSR and therefore not natively supported by guzzle. i just read laravel/socialite#468 - some thoughts: the goal of PSR-18 is to provide flexibility. a package depending on a specific implementation is against the philosophy of all PSR standards. only applications should decide which implementation they use. if i understand correctly, your worry is that the user of laravel/socialite will have to chose which PSR-18 implementation to use. my suggestion would be to say: "To use socialite, you need an HTTP client. We recommend installing it with |
|
Can this be merged and tagged? |
no it can't. see the comments above yours for the reasons. we'd need someone to volunteer to build an async adapter for guzzle 7+. there is no point in having an adapter for sync guzzle 7+, because that implements psr natively. |
|
I first saw this PR and thought it was strange that we needed this. But with the Async argument I think it makes sense. I vote for a guzzle7-adapter package. @dbu are you okey with that? Can I start working on that repo? |
i would love to have async in the name here, to be explicit that we do not provide a PSR adapter anymore, only a httplug adapter for async. not sure about |
|
The PSR argument is invalid as far as I can see. Lots of packages depend on HTTPlug which predates PSR-18, so projects which use those packages can't update to Guzzle 7 and use it with HTTPlug. |
hm, i guess that is a good point. lets do guzzle7-adapter then. if we also do the sync client, chances are that "something" changes with guzzle 8 that would make it difficult to handle both. |
|
Could someone confirm that this package is working before I tag a release? |
The README still says "Guzzle 6 HTTP Adapter". 😉 |
|
Thank you for the PR. I'll close this PR because we got a Guzzle7 package now. |
|
Guzzle7 adapter still needs to be added to the Discovery, right? |
|
yeah it would make sense to add it to discovery. want to do a pull request for that @barryvdh ? |
|
Yes done php-http/discovery#189 |
What's in this PR?
Guzzle 7.0 is almost there, and as it's almost the same as Guzzle 6, the adapter should just support them both versions
Why?
To be able to use the new version via the adapter
Example Usage
Checklist
CHANGELOG.md