Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple from symfony/http-client #1318

Open
ostrolucky opened this issue Oct 21, 2022 · 10 comments
Open

Decouple from symfony/http-client #1318

ostrolucky opened this issue Oct 21, 2022 · 10 comments

Comments

@ostrolucky
Copy link

This is a continuation of #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
@nicolas-grekas
Copy link
Contributor

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
Copy link
Contributor

nicolas-grekas commented Oct 21, 2022

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
Copy link
Author

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
Copy link
Contributor

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

@jderusse
Copy link
Member

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
Copy link
Member

stof commented Oct 21, 2022

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
Copy link
Contributor

GromNaN commented Oct 22, 2022

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. php-http/httplug#165 (comment)

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
Copy link
Member

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
Copy link
Contributor

GromNaN commented Nov 3, 2022

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants