search

lostisland / faraday

Simple, but flexible HTTP client library, with support for multiple backends.
https://lostisland.github.io/faraday
MIT License
5.76k stars 981 forks source link

Async::HTTP::Faraday cannot use "in_parallel" #1583

Closed ioquatix closed 3 months ago

ioquatix commented 3 months ago

We need to wrap the requests with Async{} but the parallel manager is only called AFTER the fact.

https://github.com/lostisland/faraday/blob/3efc0a89825da053e7beb85b9b264f424df0e893/lib/faraday/connection.rb#L324C1-L325C29

In other words, it would be better:

parallel_manager&.run(&block)
iMacTia commented 3 months ago

I'm not a fan myself of the current interface, but as it was pointed out in this comment it is actually possible to deal with this by using the parallel? test method in the adapter, collecting all the requests in an array, and postponing the execution until run is called.

Although I agree with you parallel_manager&.run(&block) would be a much better interface, making this change would break existing implementations 😞 so right now I don't think it's an option for us

ioquatix commented 3 months ago

We could sniff whether run takes an explicit block:

e.g.

parallel_manager.method(:run).parameters.empty?

That would make it an option to make the proposed change parallel_manager&.run(&block)

Another option is to introduce a different method and use respond_to?.

iMacTia commented 3 months ago

Unorthodox, but it would work. I'm concerned by the parameters approach though, would rather have a different method and check with respond_to?

iMacTia commented 3 months ago

@ioquatix take a look at #1584 when you get a chance. Would you be able to test the new method before I cut a new release?

iMacTia commented 3 months ago

@ioquatix FYI – just released Faraday 2.11.0 with support for the new #execute method 👍

ioquatix commented 3 months ago

Thanks for working with me on this new feature!