-
Notifications
You must be signed in to change notification settings - Fork 35
Description
While working on #122 I started wondering if Link#http_method
was correct when using the Futuroscope::Future
:
def http_method(method, body = nil)
@resource = begin
response =
if @entry_point.options[:async]
Futuroscope::Future.new do
@entry_point.connection.run_request(method, _url, body, nil)
end
else
@entry_point.connection.run_request(method, _url, body, nil)
end
Resource.new(response.body, @entry_point, response)
end
end
More specifically, we create a new Futuroscope::Future
, store it in response
, but then immediately ask for it's resource.body
when creating a new resource. This means that we basically block the current thread while waiting for the future to resolve so we can call body
on it. After the request finishes executing in the background, we finally get the body
, and are able to create an return the new Resource
.
Looking at git history, this has been broken since the Resource
was introduced in 5de6f84, which was released in version 0.6 in 2014.
So this means that for all of almost three years nobody really noticed that this was broken, which leads me to make some observations:
- Would it make sense for
async: false
to be the default behavior instead, since that's how people have been using it for a long time? - Would it make sense to consider removing the background/async functionality altogether?
- Alternatively, would it make sense to consider replacing futuroscope using the
concurrent-ruby
which has been adopted by the likes ofactivesupport
,dry-*
,graphql
,hanami
,rom
,sidekiq
, etc. and thus most ruby applications probably already have it as a direct or indirect dependency?