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

Add support for a new ParallelManager#execute method. #1584

Merged
merged 2 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ Metrics/AbcSize:
# Offense count: 3
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 225
Max: 230

# Offense count: 9
# Configuration parameters: AllowedMethods, AllowedPatterns.
Expand Down
30 changes: 23 additions & 7 deletions docs/adapters/custom/parallel-requests.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,34 @@ class FlorpHttp < ::Faraday::Adapter
def self.setup_parallel_manager(_options = nil)
FlorpParallelManager.new # NB: we will need to define this
end
end

class FlorpParallelManager
def add(request, method, *args, &block)
# Collect the requests
def call(env)
# NB: you can call `in_parallel?` here to check if the current request
# is part of a parallel batch. Useful if you need to collect all requests
# into the ParallelManager before running them.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This context-giving comment is gold.

end
end

def run
# Process the requests
class FlorpParallelManager
# The execute method will be passed the same block as `in_parallel`,
# so you can either collect the requests or just wrap them into a wrapper,
# depending on how your adapter works.
def execute(&block)
run_async(&block)
end
end
```

Compare to the finished example [em-synchrony](https://github.com/lostisland/faraday-em_synchrony/blob/main/lib/faraday/adapter/em_synchrony.rb)
### A note on the old, deprecated interface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and so good with this note!


Prior to the introduction of the `execute` method, the `ParallelManager` was expected to implement a `run` method
and the execution of the block was done by the Faraday connection BEFORE calling that method.

This approach made the `ParallelManager` implementation harder and forced you to keep state around.
The new `execute` implementation allows to avoid this shortfall and support different flows.

As of Faraday 2.0, `run` is still supported in case `execute` is not implemented by the `ParallelManager`,
but this method should be considered deprecated.

For reference, please see an example using `run` from [em-synchrony](https://github.com/lostisland/faraday-em_synchrony/blob/main/lib/faraday/adapter/em_synchrony.rb)
and its [ParallelManager implementation](https://github.com/lostisland/faraday-em_synchrony/blob/main/lib/faraday/adapter/em_synchrony/parallel_manager.rb).
14 changes: 11 additions & 3 deletions lib/faraday/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -314,15 +314,23 @@ def in_parallel?
#
# @yield a block to execute multiple requests.
# @return [void]
def in_parallel(manager = nil)
def in_parallel(manager = nil, &block)
@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")
nil
end
yield
@parallel_manager&.run
return yield unless @parallel_manager

if @parallel_manager.respond_to?(:execute)
# Execute is the new method that is responsible for executing the block.
@parallel_manager.execute(&block)
else
# TODO: Old behaviour, deprecate and remove in 3.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Very kind not to emit deprecation warnings, here!)

yield
@parallel_manager.run
end
ensure
@parallel_manager = nil
end
Expand Down