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

Allow running Que on Rails 7.1 #401

Closed
wants to merge 2 commits into from

Conversation

texpert
Copy link
Contributor

@texpert texpert commented Oct 9, 2023

QueAdapter has been removed from ActiveJob in Rails 7.1, so now a Rails 7.1 app fails to start with the cannot load such file -- active_job/queue_adapters/que_adapter (LoadError) even if not using ActiveJob with Que.

This PR is enabling running Que on Rails 7.1 not requiring que/active_job/extensions.

Copy link
Member

@ZimbiX ZimbiX left a comment

Choose a reason for hiding this comment

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

Sorry, I didn't realise this would be an issue in removing the Que adapter from Rails! 😬 I really should have tested that before giving the go ahead for rails/rails#46005 in rails/rails#45899.

Thanks for having a go at fixing this. However, according to Que's tests (which it turns out has incorrect gem version specificity, so Que's tests against Rails 7.0 are actually testing 7.1), it looks like this change hasn't fixed the issue:

<internal:/opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require': cannot load such file -- active_job/queue_adapters/que_adapter (LoadError)
	from <internal:/opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from /opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/gems/3.2.0/gems/activejob-7.1.0/lib/active_job/queue_adapters.rb:138:in `const_get'
	from /opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/gems/3.2.0/gems/activejob-7.1.0/lib/active_job/queue_adapters.rb:138:in `lookup'
	from /opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/gems/3.2.0/gems/activejob-7.1.0/lib/active_job/queue_adapter.rb:51:in `queue_adapter='
	from /home/runner/work/que/que/spec/spec_helper.rb:13:in `<top (required)>'
	from <internal:/opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from <internal:/opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from /home/runner/work/que/que/spec/que/active_job/extensions.job_extensions_spec.rb:3:in `<top (required)>'
	from <internal:/opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from <internal:/opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from /opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:21:in `block in <main>'
	from /opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in `select'
	from /opt/hostedtoolcache/Ruby/3.2.2/x64/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in `<main>'

I read the logic you've added as 'skip the require on Rails 7.1' - I think whatever fix we work out should apply to Rails 7.1+, not just 7.1.

Also, I don't think requiring that file is the issue. I think the extensions are still needed... This autoloading of a nonexistent class might be more the issue.

@ZimbiX ZimbiX force-pushed the allow-running-with-rails-7-1 branch from fb421bf to a0d24c4 Compare October 9, 2023 13:41
@ZimbiX
Copy link
Member

ZimbiX commented Oct 9, 2023

I've fixed the tests' Rails version specificity in #402, and rebased this PR, also adding testing against Rails 7.1.

@ZimbiX
Copy link
Member

ZimbiX commented Oct 9, 2023

I've taken a different direction, so thought that warranted a new PR =) #403

@ZimbiX ZimbiX closed this Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants