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

[Railties] Add config rake_eager_load #28209

Merged

Conversation

tjoyal
Copy link
Contributor

@tjoyal tjoyal commented Feb 27, 2017

Summary

Rake environment disable config.eager_load for from what I understand legacy reasons.

https://github.com/rails/rails/blob/v5.0.1/railties/lib/rails/application.rb#L446

I would wish to be able to control that behaviour so regardless how the application is accessed, the load order remains the same.

I added a new accessor in Rails::Application::Configuration to minimize the diff, but it could be nice to split the mass creation to one per line and 🔡 ?

Copy link
Member

@guilleiguaran guilleiguaran left a comment

Choose a reason for hiding this comment

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

👍

@kirs
Copy link
Member

kirs commented Feb 27, 2017

Looks as a great path to start towards deprecating the old behaviour 👍

@olivierlacan
Copy link
Contributor

@tjoyal Excellent PR. The legacy reason was bad interaction with rake assets precompile I believe, where a database connection was attempted due to eager loading.

I would make a bigger deal about this in the changelog if I were you since the previous change to make rake task never eager load was a sizable regression that could cause very problematic issues with thread safety in rake tasks.

@@ -1,3 +1,11 @@
* Allow configuration of eager_load behaviour for rake environment:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say "class eager_load` behavior" here to avoid potential confusions with ActiveRecord eager loading. I get that we're not in the AR changelog but you never know. :-p

@rafaelfranca
Copy link
Member

For now we don't want to advertise this feature too much since it is going to be just a workaround to get it shipped in Rails 5.1 and that will be removed in 5.2. In the long term all the rake tasks will have eager loading disabled and we will give users a mechanism to eager load the tasks they want. We are also going to kill most of the Rake tasks in favor of Rails commands that will respect the configurations.

@seandilda
Copy link

What's the current status on this? I'm still seeing eager_load problems with tasks in 5.2

@ghost
Copy link

ghost commented Aug 29, 2018

@seandilda - same here, we'd also like eager loading to be configurable in rake tasks

@randoum
Copy link

randoum commented Jan 30, 2019

For now we don't want to advertise this feature too much since it is going to be just a workaround to get it shipped in Rails 5.1 and that will be removed in 5.2. In the long term all the rake tasks will have eager loading disabled and we will give users a mechanism to eager load the tasks they want. We are also going to kill most of the Rake tasks in favor of Rails commands that will respect the configurations.

@rafaelfranca Almiost 2 years since this tiny PR is pending. Though it would be very helpful to have it merged. Are you planning to go through this?

@rafaelfranca rafaelfranca added this to the 6.0.0 milestone Feb 4, 2019
@kaspth kaspth modified the milestones: 6.0.0, 6.1.0 Apr 17, 2019
@rails-bot
Copy link

rails-bot bot commented Dec 18, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants