-
Notifications
You must be signed in to change notification settings - Fork 462
Description
We recently attempted to add the faraday-net_http_persistent adapter to our Koala stack and noticed that it wasn't behaving as expected.
After tracing it down, I don't believe Koala is compatible with this adapter because Koala::HTTPService.make_request
will create a new Faraday instance for every request:
koala/lib/koala/http_service.rb
Lines 49 to 50 in c327913
# set up our Faraday connection | |
conn = Faraday.new(request.server, faraday_options(request.options), &(faraday_middleware || DEFAULT_MIDDLEWARE)) |
In doing this, a new connection pool is created each time so there's nothing to share between requests.
I don't believe this is trivial change as the way around it is to create a Faraday connection first (so the middleware stack is built once), and then invokes requests on that connection (a bit similar to the example here). That might be a bit too breaking depending on how folks are using this library 🤔
koala/lib/koala/api/graph_api_methods.rb
Lines 54 to 57 in c327913
def get_object(id, args = {}, options = {}, &block) | |
# Fetches the given object from the graph. | |
graph_call(id, args, "get", options, &block) | |
end |
Lines 46 to 49 in c327913
def graph_call(path, args = {}, verb = "get", options = {}, &post_processing) | |
# enable appsecret_proof by default | |
options = {:appsecret_proof => true}.merge(options) if @app_secret | |
response = api(path, args, verb, options) |
Lines 121 to 122 in c327913
# make the request via the provided service | |
result = Koala.make_request(path, args, verb, options) |
Lines 60 to 63 in c327913
# An convenenient alias to Koala.http_service.make_request. | |
def self.make_request(path, args, verb, options = {}) | |
http_service.make_request(HTTPService::Request.new(path: path, args: args, verb: verb, options: options)) | |
end |
koala/lib/koala/http_service.rb
Lines 48 to 50 in c327913
def self.make_request(request) | |
# set up our Faraday connection | |
conn = Faraday.new(request.server, faraday_options(request.options), &(faraday_middleware || DEFAULT_MIDDLEWARE)) |