-
Notifications
You must be signed in to change notification settings - Fork 363
Add a way to temporarily opt-out to #all now lazily making requests:
#460
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
base: main
Are you sure you want to change the base?
Conversation
Closes rails#460 Introduce both the `config.active_resource.lazy_collections` configuration, as well as the `Base.lazy_collections` class attribute. Restore the original behavior of immediately retrieving collections by defaulting the initial value to `false`, but deprecate the behavior and encourage migrating to setting the value to `true`.
Closes rails#460 Introduce both the `config.active_resource.lazy_collections` configuration, as well as the `Base.lazy_collections` class attribute. Restore the original behavior of immediately retrieving collections by defaulting the initial value to `false`, but deprecate the behavior and encourage migrating to setting the value to `true`.
Closes rails#460 Introduce both the `config.active_resource.lazy_collections` configuration, as well as the `Base.lazy_collections` class attribute. Restore the original behavior of immediately retrieving collections by defaulting the initial value to `false`, but deprecate the behavior and encourage migrating to setting the value to `true`.
|
Should this commit mention the behavior and configuration in the README or somewhere else? I had drafted some changes as part of #461: Collections are lazily loaded by default, deferring the HTTP request until its items are first accessed.: # Build a lazy collection with initial query parameters
people = Person.where(first_name: "Tyler")
# Modify the lazy collection with additional query parameters
people = people.where(last_name: "Durden")
# Expects a response of
#
# [
# {"id":1,"first_name":"Tyler","last_name":"Durden"},
# ]
#
# for GET http://api.people.com:3000/people.json?first_name=Tyler&last_name=Durden
people.first # => <Person::xxx 'first_name' => 'Tyler' ...>Set Person.lazy_load_collections = false
# Expects a response of
#
# [
# {"id":1,"first_name":"Tyler","last_name":"Durden"},
# ]
#
# for GET http://api.people.com:3000/people.json?first_name=Tyler
people = Person.where(first_name: "Tyler")
# Access items from the eagerly-loaded collection
people.first # => <Person::xxx 'first_name' => 'Tyler' ...>Support for lazily loaded collections is deprecated, and will be removed in a future release. |
|
Another plan is to allow this to be configured, and deprecate setting the config to disable the lazy load. I'm ok with that too. |
|
Yeah I think deprecating the flag assignment may be less noisy (although we lose the ability to track down callers in production that may be not covered in tests). I'll make the change. Also, FYI @seanpdoyle, I realized there is another change related to the class BillingAPI < ActiveResource::Base
self.collection_parser = CustomCollection
end
# Before
BillingAPI.all #=> Returns a `CustomCollection`
# After
# We have no way to return the collection_parser object itself. We can only call Enumerable methods on it. |
Follow-up to rails#460 Expose the underlying `ActiveResource::Collection` instance for the `WhereClause` class returned from `Base.all` and `Base.where`. To do so, rename the previously private `resources` method to `collection`, and move it to a public declaration.
I've opened #462 to resolve that issue. It's a bit of a band-aid, since I think a better solution would be to change how |
- ### Problem In 9372d5a we made a change that defer the http request submission until the collection is used. ```ruby # Before Product.all #=> HTTP request is made # After products = Product.all #=> No HTTP request is made products.each { ... } #=> HTTP request is made here ``` This create issues for codepaths that used to expect the `Product.all` to make the http request and this feels like a breaking change that should go through a deprecation cycle (too late now but let's have a way to opt-out to the new behaviour). ### Context We have a safety net in our application that does something like this: ```ruby products = allow_explicit_http_connections do Product.all end products.each { ... } ``` This is now broken because we make the request outside of the explictly allowed resiliency block. ### Solution Introduce a way to opt out to the new behaviour and trigger a deprecation warning.
Add a way to temporarily opt-out to
#allnow lazily making requests:Problem
In 9372d5a we made a change that defer the http request submission until the collection is used.
This create issues for codepaths that used to expect the
Product.allto make the http request and this feels like a breaking change that should go through a deprecation cycle (too late now but let's have a way to opt-out to the new behaviour).Context
We have a safety net in our application that does something like this:
This is now broken because we make the request outside of the explictly allowed resiliency block.
Solution
Introduce a way to opt out to the new behaviour and trigger a deprecation warning.
cc/ @seanpdoyle