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

http client recommendation: Symfony? #611

Closed
tacman opened this issue Jan 16, 2024 · 14 comments
Closed

http client recommendation: Symfony? #611

tacman opened this issue Jan 16, 2024 · 14 comments
Labels
documentation Improvements or additions to documentation

Comments

@tacman
Copy link

tacman commented Jan 16, 2024

we recommend using Guzzle 7:

FWIW, I like the Symfony client better. I believe it's faster, and a very clean API (via symfony/contracts), follows the PSR standards and has built-in error logging, async and retries.

Furthermore, anyone using the symfony framework gets http requests in the debug toolbar.

I'm glad installing Symfony is documented, but in 2024 I think it's also the better option and would recommend that it be the recommended way, and then showing an example with Guzzle 7.

@norkunas
Copy link
Collaborator

norkunas commented Jan 16, 2024

In meilisearch-symfony lib of course it uses Symfony HttpClient:) if you have it

@curquiza
Copy link
Member

I close this issue then. Feel free to ask to re-open if needed 😊

@tacman
Copy link
Author

tacman commented Feb 13, 2024

Again, this is just a request to recommend Symfony rather than Guzzle in the documentation. I can make the PR if you'll accept it, but are you saying you prefer to recommend Guzzle?

I understand that the bundle using Symfony if you have it.

@curquiza
Copy link
Member

curquiza commented Feb 13, 2024

Sorry I went to fast and I was confused in my open tabs 😅 did not answer the right one!

@norkunas do you think we should recommend Symfony bundler instead?
We did not have any issue with recommending Guzzle so far, and we also mention the Symfony ones in the example right below in the README, so I personally don't see the point of changing this, but I might be wrong

@curquiza curquiza reopened this Feb 13, 2024
@norkunas
Copy link
Collaborator

As I come from Symfony ecosystem I'd recommend it's http client, but it's a biased opinion. Laravel ecosystem uses mostly guzzle and guzzle has more downloads, but that doesn't mean it's better than symfony http client. I'd remove the recommendation at all and instead just would leave the link to https://packagist.org/providers/psr/http-client-implementation so people can choose what they need.
Another option reword the examples:
If you want to use guzzle, do this: .., if you want to use symfony http client: do this ..

@tacman
Copy link
Author

tacman commented Feb 13, 2024

Or maybe just offer both equally, rather than a recommendation, based on your needs.

https://stackshare.io/stackups/guzzle-vs-symfony#:~:text=Symfony's%20HttpClient%20also%20provides%20support,request%20and%20response%20handling%20process.

This is an interesting read, and it's hard to tell if it's still accurate in 2024, since both clients have improved.

All my projects are in Symfony, but many bundles require guzzle, leading to an extra configuration,e.g. KnpLabs/packagist-api#91

Fortunate, this library doesn't do that, so really it's just a matter of why encourage one over the other.

@norkunas
Copy link
Collaborator

Symfony's HttpClient, on the other hand, does not provide built-in support for concurrency and is focused more on simplicity and ease of use.

bullshit :D from sf docs:

Concurrent Requests
Thanks to responses being lazy, requests are always managed concurrently. On a fast enough network, the following code makes 379 requests in less than half a second when cURL is used:

yes, sometimes there's no way to avoid additional deps because of some packages, but if you use very little of it's apis then you can just manually replace those calls directly with a http client.. but luckily we don't suffer from that problem here 🙂

@curquiza
Copy link
Member

Another option reword the examples:
If you want to use guzzle, do this: .., if you want to use symfony http client: do this ..

let's do with this

@curquiza curquiza added the documentation Improvements or additions to documentation label Feb 13, 2024
@pkruithof
Copy link

I'd remove the recommendation at all and instead just would leave the link to https://packagist.org/providers/psr/http-client-implementation so people can choose what they need.

This is a good idea IMO, and I think you're already nearly there, since I see the Psr\Http\* interfaces being used. The only thing I would add is to remove the php-http/* packages, or at least these two. Requiring a Psr17/Psr18 implementation pretty much removes the need for this, as long as you have dependency injection set up.

@norkunas
Copy link
Collaborator

norkunas commented Jul 1, 2024

@pkruithof I suggested removal of php-http/* packages in #653

@curquiza
Copy link
Member

curquiza commented Aug 2, 2024

What do we miss to close this issue @norkunas?

@norkunas
Copy link
Collaborator

norkunas commented Aug 2, 2024

Not sure. Can be closed or we can recommend symfony http client, up to you.

@curquiza
Copy link
Member

curquiza commented Aug 2, 2024

Recommending the symfony client is a really tiny detail, and up to any developer to choose.
As long as we show symfony client works with this package, I'm ok with the current README.
Closing this issue then

@curquiza curquiza closed this as completed Aug 2, 2024
@tacman
Copy link
Author

tacman commented Aug 2, 2024

we recommend using Guzzle 7:

Why?

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

No branches or pull requests

4 participants