search

async-aws / aws

AWS SDK with readable code and async responses
https://async-aws.com
MIT License
428 stars 128 forks source link

Decouple from symfony/http-client #1318

Open ostrolucky opened 1 year ago

ostrolucky commented 1 year ago

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:

nicolas-grekas commented 1 year ago

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 commented 1 year ago

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

ostrolucky commented 1 year ago

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

nicolas-grekas commented 1 year ago

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

jderusse commented 1 year ago

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.

stof commented 1 year ago

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)

GromNaN commented 1 year ago

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.

jderusse commented 1 year ago

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

GromNaN commented 1 year ago

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

nicolas-grekas commented 1 year ago

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

SamMousa commented 1 week ago

FWIW, the https://github.com/FriendsOfPHP/well-known-implementations repo has been archived. So it seems we won't have this for a bit.

I've created this adapter: https://github.com/SAM-IT/symfony-http-psr18 and use it in production. This is not async, but I don't see the need for it in most use cases: