Skip to content

Conversation

@simonberger
Copy link
Contributor

@simonberger simonberger commented Mar 15, 2023

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? yes
Tickets Fix #49644
License MIT
Doc PR symfony/symfony-docs#18064

This MR does 2 closely related things to try to fully integrate the HttplugClient with the unique features it brings and on the other hand deprecate its use like a psr18 client which it is in a pending deprecation state since a long time.

  • Added a new services httplug.http_client and alias Http\Client\HttpAsyncClient to inject the the HttplugClient.
  • Make the available service Http\Client\HttpClient a deprecated alias of it
  • Create httplug.<scoped_client_id> services for all scoped clients like it is done with the psr18 ClientInterface

@carsonbot carsonbot added this to the 6.3 milestone Mar 15, 2023
@simonberger simonberger changed the title Add scoped httplug clients and deprecate httplugs use like psr18 client [HttpClient] Add scoped httplug clients and deprecate httplugs use like psr18 client Mar 15, 2023
@nicolas-grekas
Copy link
Member

Thanks for working on this.
I wouldn't deprecate the method (deprecating the auto wiring alias is great)

@simonberger
Copy link
Contributor Author

simonberger commented Mar 15, 2023

It was my last iteration step as well. My thought process was, if I deprecate the service meant to use this method I kinda deprecate its right to exist as well and should make that official.
Maybe an argument to leave it in place is that the method does not hurt enough to compensate the deprecation hassle and removal of the HttpClient interface of this class with the 7.0 release.

My idea is to move the HttplugClient away from its superseded psr18 client use to its valuable promise response feature until there is maybe a psr replacement for it (which currently does not look like it will ever happen) or a better semi official implementation.
I like to change the documentation in this regard as well.

@simonberger simonberger force-pushed the add-scoped-httplug-clients branch from d378946 to a7b11f8 Compare March 15, 2023 14:20
@nicolas-grekas
Copy link
Member

there should be also no HttpClient service deprecation then as this would actually disable injecting httplug with this use case anyway

That's two very different things to me. Removing an autowiring alias just sends a message saying that in the context of symfony apps (where we have some authority), we're discouraging using this interface. On the broader php ecosystem, we don't, and thus we can't.

@simonberger
Copy link
Contributor Author

there should be also no HttpClient service deprecation then as this would actually disable injecting httplug with this use case anyway

That's two very different things to me. Removing an autowiring alias just sends a message saying that in the context of symfony apps (where we have some authority), we're discouraging using this interface. On the broader php ecosystem, we don't, and thus we can't.

Ok got it, I have to see the remaining use outside of a symfony bundle integration. 👍

@simonberger simonberger force-pushed the add-scoped-httplug-clients branch from 5e20465 to 5c81cb4 Compare March 15, 2023 15:34
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add two (short) lines in the changelog of FrameworkBundle: one for the new aliases, one for the deprecation

@carsonbot carsonbot changed the title [HttpClient] Add scoped httplug clients and deprecate httplugs use like psr18 client [FrameworkBundle] Add scoped httplug clients and deprecate httplugs use like psr18 client Mar 15, 2023
@nicolas-grekas nicolas-grekas force-pushed the add-scoped-httplug-clients branch from bec8ee7 to 20ab567 Compare March 16, 2023 10:04
@nicolas-grekas
Copy link
Member

Thank you @simonberger.

@nicolas-grekas nicolas-grekas merged commit 67d8daa into symfony:6.3 Mar 16, 2023
@fabpot fabpot mentioned this pull request May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Status of the HttplugClient / Lacking scoped clients integration

4 participants