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

NoMethodError: undefined method `active_job' for Rails::Application::Configuration #35782

Closed
kivanio opened this issue Mar 28, 2019 · 14 comments

Comments

@kivanio
Copy link
Contributor

commented Mar 28, 2019

Steps to reproduce

I have an app where I do not need Active Job, so its commented at application.rb:

# require "active_job/railtie"

In last version of rails 5.2.3 errors appear every time I ran: bundle exec rake db:setup.
I believe it is because this commit:
44133fd

rake aborted!
NoMethodError: undefined method `active_job' for #
/home/runner/bole.to/vendor/bundle/ruby/2.5.0/gems/railties-5.2.3/lib/rails/railtie/configuration.rb:97:in `method_missing'
/home/runner/bole.to/vendor/bundle/ruby/2.5.0/gems/railties-5.2.3/lib/rails/engine.rb:662:in `with_inline_jobs'
/home/runner/bole.to/vendor/bundle/ruby/2.5.0/gems/railties-5.2.3/lib/rails/engine.rb:551:in `load_seed'
/home/runner/bole.to/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.3/lib/active_record/tasks/database_tasks.rb:281:in `load_seed'
/home/runner/bole.to/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.3/lib/active_record/railties/databases.rake:194:in `block (2 levels) in '
/home/runner/bole.to/vendor/bundle/ruby/2.5.0/gems/rake-12.3.2/exe/rake:27:in `'
/home/runner/.rbenv/versions/2.5.1/bin/bundle:23:in `load'
/home/runner/.rbenv/versions/2.5.1/bin/bundle:23:in `'

Expected behavior

When require is not commented:
Loading development environment (Rails 5.2.3)
[1] pry(main)> Rails.application.config.active_job.queue_adapter
=> :async

Actual behavior

When require is commented:

Loading development environment (Rails 5.2.3)
[1] pry(main)> Rails.application.config.active_job.queue_adapter
NoMethodError: undefined method `active_job' for #
from /Users/kivanio/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/gems/railties-5.2.3/lib/rails/railtie/configuration.rb:97:in `method_missing'

System configuration

Rails version:
5.2.3

Ruby version:
2.5.1

@rafaelfranca

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

active_job is not an optional dependency. We don't support to comment the railtie since other framework depend on it.

@rafaelfranca

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Actually, while it is not optional right now, I think it could be. @matthewd do you remember why we didn't make active job optional?

@kivanio

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

Hi
If you do not think it's a bug, you can close it.
if this change were made on rails 6.0
I would have no problem accepting it.
But in a small version, making a change that breaks things does not make sense to me.
The application has been running since rails 3 and has never used Active Job.
So I thought it was a bug.
Thanks for clarifying.

@jlw

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2019

This absolutely is a bug to introduce a breaking change in a patch version of Rails.

Further, it is preposterous to think that ActiveJob is a required framework. Sure, all of the apps you work on may have a good use case for queued background work, but that just means you're an expert in a particular niche.

@sikachu

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

Hello,

I don't think it was really @rafaelfranca or any contributors intention to introduce a breaking change in a patch release. This is an unintentional side effect of merging 44133fd in ...

But yeah, as @rafaelfranca pointed out — we didn't make Active Job as an optional dependency, but it just happen to be that you could exclude Active Job and the app was still working.

So, given that (1) you could use Rails without Active Job before 2.5.3 (2) Active Job is not a declared dependency on railtie gem, I think I'm leaning toward fixing that code to be this:

if config.respond_to?(:active_job)
  queue_adapter = config.active_job.queue_adapter

  ActiveSupport.on_load(:active_job) do
    self.queue_adapter = :inline
  end
end

That way we can allow people to still use Rails without Active Job and not introduce an unintended regression as a stop gap. Then, we can properly introduce a test and documentation to sanction that Rails app can run without Active Job if you choose to do so in the future versions.

What do you think about my proposal?

(I really wish we found out about this during RC period ... T_T)

@jlw

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2019

@sikachu, I was considering something like this:

def load_seed
  seed_file = paths["db/seeds.rb"].existent.first
  return unless seed_file

  if config.respond_to?(:active_job)
    with_inline_jobs { load(seed_file) }
  else
    load(seed_file)
  end
end
@rafaelfranca

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

I'd keep the conditional closer to the usage. @jlw do you want to open a PR?

@jlw

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2019

@rafaelfranca - sure, I'd love to put together a PR on this - but this is my first time working on Rails directly and I imagine it will be Monday at the earliest.

@matthewd

This comment has been minimized.

Copy link
Member

commented Mar 30, 2019

@rafaelfranca Active Job is optional -- but it's a required dependency of Action Mailer (and Active Storage, I imagine, thought that seems to be missing the gem dep? cc @georgeclaghorn). It's required there, even if you don't want to do the work in a separate process, because AJ is the abstraction layer you use to express that preference (by using the inline adapter).

Commenting out the railtie is definitely supposed to work -- if you do that and then use actionmailer, it'll require it for you.

We should probably add some railties tests that do a basic generate-setup-and-boot with every combination of frameworks commented, in hope of catching this sort of thing... though as the main integration point with all the frameworks coming together, it's always going to be the hardest part to effectively cover.

jlw added a commit to jlw/rails that referenced this issue Apr 2, 2019

@jlw

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

@matthewd or @rafaelfranca - I am having trouble creating test that fails either on master or on the 5-2-stable branch. I'll continue to try and find differences between our production app that ran into this problem and the test dummy app template, but do you have any suggestions that stand out when looking at the temp commit linked above?

@rafaelfranca

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

First thing I see is that you are testing in the wrong file. Engine.rb is for rails engines, you should be testing in the app. But other than that I'd expect that to fail. Maybe the application that is being booted is not the one were you removed the active job railtie?

@jlw

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

@rafaelfranca - two things:

First, the bug was introduced in engine.rb which is why I am testing/fixing there. Gannon McGibbon only added ActiveJob processing of seeds within engines, so testing a change within the app doesn't make sense to my understanding. Could you explain a bit more what you have in mind so that I can understand it better?

Second, I was hoping that this change would be applied to any future Rails 5.2 bug fix releases. I made use of a modified version of the test file's restrict_frameworks which does not exist in the 5-2-stable branch, but that's probably not necessary (based on the logic of my bug fix, overriding the config alone should be sufficient). What is the best way to get this bug fix included both in future 5.2.x and 6.x releases? Are there special tags or branches I should be using with my commit/PR?

gmcgibbon added a commit that referenced this issue Apr 20, 2019

Merge pull request #35896 from jlw/bug/active-jobless-seeds
[#35782] Allow loading seeds without ActiveJob (~> 5.2.3)

nickcharlton added a commit to thoughtbot/administrate that referenced this issue Apr 23, 2019

Drop active_job from the dependencies.
In #8bc80cf, we needed to add `active_job` to the dependencies to allow
database seeding to continue to work. This was raised in
rails/rails#35782
and fixed in
rails/rails@d3dc651

nickcharlton added a commit to thoughtbot/administrate that referenced this issue Apr 23, 2019

Drop active_job from the dependencies.
In 8bc80cf, we needed to add `active_job`
to the dependencies to allow
database seeding to continue to work. This was raised in
rails/rails#35782
and fixed in
rails/rails@d3dc651
@phillc

This comment has been minimized.

Copy link

commented Apr 24, 2019

Hello,

This change has caused my seeds to execute jobs that do not execute under rails 5.2.2.

I use delayed job, and in my seeds I create some data, and then move long running tasks (like api requests) onto the queue.

This change has forced these jobs to begin executing immediately. I have one project that relies on this backgrounding to do several long tasks in parallel, and another that utilizes delayed job to clear specific jobs from the db (this issue has surfaced because that project uses webmock to make sure seeds don't hit external APIs).

On top of that, if in db/seeds.rb I inspect the current queue adapter via ApplicationJob.queue_adapter or Rails.application.config.active_job.queue_adapter, I am told that it is delayed_job. Forcing an exception in the job shows a trace that somehow the inline adapter taking over... Only when inspecting ApplicationJob.queue_adapter from within the job do I finally see inline is set. It has taken some time to find this execution path.

I am still trying to wrap my head around the root issue above. Perhaps if this is entirely a work around of not calling active job if it isn't there, perhaps the check should be simply to check if it is there, and to utilize whatever queue adapter is already set instead of forcing inline.

sihugh added a commit to alphagov/collections-publisher that referenced this issue Apr 26, 2019

Add active_job requirement
Apparently this is required to be loaded: rails/rails#35782

gmcgibbon added a commit that referenced this issue May 7, 2019

Merge pull request #35896 from jlw/bug/active-jobless-seeds
[#35782] Allow loading seeds without ActiveJob (~> 5.2.3)
@franzliedke

This comment has been minimized.

Copy link

commented Jul 22, 2019

Hi folks, is there a timetable for the next patch release for Rails 5.2?

Apparently, this regression has been fixed. Without it, our seeding is completely broken, which is a nuisance. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.