aws icon indicating copy to clipboard operation
aws copied to clipboard

Decouple from symfony/http-client

Open ostrolucky opened this issue 3 years ago • 11 comments

This is a continuation of https://github.com/async-aws/aws/issues/756

What changed since then? https://twitter.com/nicolasgrekas/status/1583454980769730560

https://github.com/FriendsOfPHP/well-known-implementations has been created which matches this use case. We can have both worlds:

  • This package doesn't have to require symfony/http-client
  • If consumer doesn't have http-client, it will be installed

ostrolucky avatar Oct 21 '22 14:10 ostrolucky

It might be hard because async is provided by symfony/http-client-contracts, and symfony/http-client is the only implementation I'm aware of.

We could add symfony/http-client-contracts to the well-known abstractions + make it install symfony/http-client when it's not found, but what would be the point when there are no alternatives?

nicolas-grekas avatar Oct 21 '22 14:10 nicolas-grekas

This would make sense if someone writes an http-client-contracts adapter for e.g. Guzzle of course.

(Note that a generic one for PSR-18 wouldn't make sense since it's not async.)

nicolas-grekas avatar Oct 21 '22 14:10 nicolas-grekas

Perhaps one of php-http/async-client-implementation could be used here? But yeah even if interface wasn't changed in this package (and indeed it won't be anytime soon as that would be a BC break), we can still add symfony/http-client-contracts to supported packages as you suggested and if somebody creates adapter for this interface in future it would be still beneficial

ostrolucky avatar Oct 21 '22 14:10 ostrolucky

Would work for me as a 1st step. Up for a PR?

nicolas-grekas avatar Oct 21 '22 15:10 nicolas-grekas

Actually, AsyncAws does not need symfony/http-client. It is "only" coupled with symfony/http-client-contracts and embraces its philosophy (async response consumed when reading it, Retrying strategy, ...)

As far as I know, there are no "real" implementation of this contract (https://packagist.org/providers/symfony/http-client-implementation), and I'm not such switching to PSR and keeping the same experience is possible.

jderusse avatar Oct 21 '22 15:10 jderusse

I'm 100% sure it is not possible to switch to PSR-18 and to keep the same experience (if you consider the concurrency part of the experience of course, but this is a given to me as this is what justifies the "async" in the project name).

Whether a symfony/http-client-implementation can be built on top of Guzzle's async API, I don't know (and I'm not interested into spending time trying it)

stof avatar Oct 21 '22 16:10 stof

I'm 100% sure it is not possible to switch to PSR-18 and to keep the same experience (if you consider the concurrency part of the experience of course, but this is a given to me as this is what justifies the "async" in the project name).

This is where the Fibers are useful: add async capacity with sync interfaces. With a fiber-based http client, like amphp/http-client v5, the same code can be synchronous or asynchronous when embedded in an event-loop. https://github.com/php-http/httplug/issues/165#issuecomment-986254714

async-aws offers more than being built of top of the async symfony/http-client. It is also used as backend for Symfony AWS adapters (Messenger, Mailer, Notifier). Due to the sync interfaces of the adapters, they does not use the full async capabilities.

Implementing symfony/http-client-contracts on top of amphp/http-client v5 would be an option. This is a hard work to do. It is very different from the current AmpHttpClient which call Loop::run() internally.

If async-aws was able to switch to a psr-18 client AND a psr-18 adapter was implemented for amphp/http-client v5, it would make things easier to use this library in a truly async context with fibers.

GromNaN avatar Oct 22 '22 21:10 GromNaN

actually symfony/messenger leverage the async behavior of async-aws https://github.com/symfony/symfony/blob/455bd4342e652047b0ffab1ba00d98a77812d8c8/src/Symfony/Component/Messenger/Bridge/AmazonSqs/Transport/Connection.php#L233

People can use this lib in an async or sync way whatever the version of php: with or without event loop ..

jderusse avatar Oct 22 '22 22:10 jderusse

Thank you @jderusse for pointing. I should have say "reactive" instead of "async". Actually, thanks to Symfony http client, you made it possible to run parallel long-polling http requests and check the status of each in a loop. The poll_timeout is required to avoid excessive cpu consumption when queues are empty. This loop have to be designed for this specific use-case. With an event-loop approach the process would wait for network event to resume the corresponding code.

Also, publishing a message is currently blocking operation: the Response object is immediately destructed.

I took SQS and messenger as an example, but the same goes for other services. It's easier to write fibers to parallelize various tasks instead of writing the code that coordinate and optimize execution (the symfony http client way).

GromNaN avatar Nov 03 '22 21:11 GromNaN

The poll_timeout is required to avoid excessive cpu consumption when queues are empty. This loop have to be designed for this specific use-case. With an event-loop approach the process would wait for network event to wake up the corresponding code.

Not arguing about the rest of your message, but this might build on false assumptions. At least here: symfony/http-client doesn't poll anything, it reacts to network events. But maybe I missed what you meant...

nicolas-grekas avatar Nov 03 '22 21:11 nicolas-grekas