Faraday::Connection#in_parallel? has inconsistent behaviour with #in_parallel

[goalaleo]

Basic Info

• Faraday Version: at least >= 0.15.4, but probably older versions as well
• Ruby Version: shouldn't matter, but I'm using 2.6.6

Issue description

If adapter is set to be an adapter that's capable of parallel requests, then calling Faraday::Connection.in_parallel will detect that. More specifically, the detecting happens in the default_parallel_manager method's call to support_parallel?

faraday/lib/faraday/connection.rb

Lines 346 to 356 in e7ab94f

	def default_parallel_manager
	@default_parallel_manager ||= begin
	adapter = @builder.adapter.klass if @builder.adapter
	if support_parallel?(adapter)
	adapter.setup_parallel_manager
	elsif block_given?
	yield
	end
	end
	end

I would assume, that the method Faraday::Connection.in_parallel? would be equivalent to calling support_parallel?, but it's not. in_parallel? checks the ivar @parallel_manager directly, which is set to a non nil value only inside the in_parallel method, and is set back to nil in the ensure block of that method. In other words, the in_parallel? method can return true only if it's called inside the in_parallel method, and it's never called there, making it useless.

How I'd like to use in_parallel?

If it's unclear whether parallel requests are supported, I'd like to do this:

if connection.in_parallel?
  connection.in_parallel(&make_requests)
else
  make_requests.call
end

The reason for wanting to check this is that if the in_parallel method is called without a parallel capable adapter, it will output warnings:

faraday/lib/faraday/connection.rb

Lines 373 to 376 in e7ab94f

	@parallel_manager = manager || default_parallel_manager do
	warn 'Warning: `in_parallel` called but no parallel-capable adapter ' \
	'on Faraday stack'
	warn caller[2, 10].join("\n")

This will create unnecessary noise .

Steps to reproduce

Use a parallel capable gem

gem "typhoeus"

Configure Faraday to use a parallel capable adapter

connection = Faraday.new do |config|
  config.adapter :typhoeus
end

Call in_parallel?

connection.in_parallel?

Expected

true

Got

false

Suggestion

Change in_parallel? to

def in_parallel?
  adapter = @builder.adapter.klass if @builder.adapterr
  adapter && support_parallel?(adapter)
end

This would make the behaviour of in_parallel and in_parallel? consistent

[iMacTia]

Hi @goalaleo and thanks for raising this.
I think there's a bit of confusion here on the different methods, and that's absolutely not your fault, as some code comments are inexact.

in_parallel?, contrary to what the description states, is supposed to be used to know if the connection is currently performing a parallel request. This is not useful in a classic CRuby, single-threaded environment, but it becomes important in a parallel one where you could potentially want to ask "is this connection running a parallel request at the moment?". Hence why it will return true only during the request.

The correct method you should be using to simply know if the connection can make a parallel request is instead support_parallel?. Now, this method accepts an adapter parameter, which you can easily pass calling the conn.adapter attr_reader (i.e. conn.support_parallel?(conn.adapter)). However, one thing to point out here is that this looks like a legacy thing, because at the beginning the adapter was not accessible this way and was mixed together with the other middleware.

Now that we isolated it and created an accessor method, we could in theory simplify the implementation of support_parallel? to work without the adapter parameter and instead fetch that directly from the connection object itself.

So in short, the implementation you proposed for in_parallel?should instead be the one for support_parallel?, while in_parallel? should remain as it is and instead we should fix the method documentation.

How does that sound?

[goalaleo]

Hi @iMacTia, thanks for the explanation! Now it makes sense to me. Your suggestions to change support_parallel? to work without explicitly passing the adapter as parameter also sounds good to me 👍