Skip to content

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Mar 6, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues #52008
License MIT

This PR adds support for amphp/http-client version 5 as a transport for our HttpClient component.

It started as a draft at nicolas-grekas#43, which helped spot that PHP is missing the capability to suspend fibers in destructors. This was reported as php/php-src#11389 and is being fixed at php/php-src#13460.

Since the fix for php-src is going to land on PHP 8.4, using amphp/http-client version 5 will require php >= 8.4.

The implementation duplicates the one we have for v4 with the needed changes to use the v5 API. The difference are not big in size of code, but they're very low level (generators vs fibers). That would be quite useless to factor IMHO.

Copy link
Member Author

@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.

PR ready.
amphp/socket issue moved to amphp/socket#114

Status: needs review

@fabpot
Copy link
Member

fabpot commented Aug 19, 2024

Thank you @nicolas-grekas.

@fabpot fabpot merged commit a44311e into symfony:7.2 Aug 19, 2024
@nicolas-grekas nicolas-grekas deleted the hc-amp3 branch August 19, 2024 09:27
@OskarStark OskarStark changed the title [HttpClient] Add support for amphp/http-client v5 [HttpClient] Add support for amphp/http-client v5 Aug 19, 2024
@jderusse
Copy link
Member

jderusse commented Aug 20, 2024

Since the fix for php-src is going to land on PHP 8.4, using amphp/http-client version 5 will require php >= 8.4.

It's still 8.1. Is it an issue? Shall we add logic to check the running PHP version ?
https://github.com/amphp/http-client/blob/d0383fa55e4deaa89d6eab23b7a8b223b2660a58/composer.json#L29

@nicolas-grekas
Copy link
Member Author

We resolve this concern with https://packagist.org/packages/symfony/amphp-http-client-meta

@xabbuh
Copy link
Member

xabbuh commented Aug 20, 2024

Do we need to document the symfony/amphp-http-client-meta package somewhere?

@nicolas-grekas
Copy link
Member Author

Not sure, for userland, I think we already do the required runtime checks.

@fabpot fabpot mentioned this pull request Oct 27, 2024
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.

5 participants