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.

Sign up for GitHub

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

Jump to bottom

Allow ActiveJob::Base.set to configure jobs when using .perform_now #43434

Merged
merged 1 commit into from
Oct 12, 2021
Merged

Allow ActiveJob::Base.set to configure jobs when using .perform_now #43434

merged 1 commit into from
Oct 12, 2021

Conversation

bensheldon
Copy link
Contributor

Summary

Closes #43376, to allow ActiveJob::Base.set to configure jobs using perform_now.

Extracts an instance method, #set (symmetrically named to the class method .set) that extracts the configuration assignment options embedded in #enqueue.

As discussed in #43376, this behavior is necessary because:

  • Existing enqueue options may be relevant to a .perform_now job that has a retry_on handler.
  • ActiveJob extensions may add additional configuration options that should be accessible to .perform_now.

@rails-bot rails-bot bot added the activejob label Oct 12, 2021
@bensheldon bensheldon mentioned this pull request Oct 12, 2021
Closed
@ghiculescu
Copy link
Member

Should perform_later do the same?

@bensheldon
Copy link
Contributor Author

@ghiculescu .perform_later already has this behavior. The implementation is slightly different because #enqueue already accepts those options as an argument.

I can go a few directions, do you have a preference?

  • Have ConfiguredJob#perform_later's implementation use the new ActiveJob::Base#set method and not use ActiveJob::Base#enqueue's options. This would make the ConfiguredJob's implementation symmetrical.
  • Have ActiveJob::Base#perform_now accept options as an argument, which would make its interface symmetrical with ActiveJob::Base#enqueue.

@ghiculescu
Copy link
Member

ghiculescu commented Oct 12, 2021 •
edited
Loading

Oh sorry I missed that @options is passed in as an argument to enqueue, I think that's fine.

rafaelfranca
rafaelfranca requested changes Oct 12, 2021
View reviewed changes
activejob/lib/active_job/core.rb Outdated Show resolved Hide resolved
activejob/test/cases/queue_naming_test.rb Outdated
job = HelloJob.set(queue: :some_queue).perform_later
assert_equal "some_queue", job.queue_name
end

test "uses queue passed to #set" do
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test removed.

activejob/test/cases/queue_priority_test.rb Outdated
job = HelloJob.set(priority: 123).perform_later
assert_equal 123, job.priority
end

test "uses priority passed to #set" do
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test removed.

activejob/lib/active_job/configured_job.rb Show resolved Hide resolved
@rafaelfranca rafaelfranca merged commit 3bfe419 into rails:main Oct 12, 2021
@bensheldon bensheldon deleted the set_perform_now branch October 12, 2021 22:11
@bensheldon bensheldon mentioned this pull request May 7, 2022
Closed
@bensheldon bensheldon mentioned this pull request Apr 26, 2023
Merged
4 tasks
nalabjp added a commit to nalabjp/solid_queue that referenced this pull request May 11, 2024
@nalabjp
Backport ActiveJob::Base.set API
926805a 
rails/rails#43434
nalabjp added a commit to nalabjp/solid_queue that referenced this pull request May 11, 2024
@nalabjp
Backport ActiveJob::Base.set API
567f111 
rails/rails#43434
nalabjp added a commit to nalabjp/solid_queue that referenced this pull request May 12, 2024
@nalabjp
Backport ActiveJob::Base.set API
bc0fdbb 
rails/rails#43434
nalabjp added a commit to nalabjp/solid_queue that referenced this pull request May 12, 2024
@nalabjp
Backport ActiveJob::Base.set API
3893db5 
rails/rails#43434
nalabjp added a commit to nalabjp/solid_queue that referenced this pull request May 12, 2024
@nalabjp
Backport ActiveJob::Base.set API
97b0a0d 
rails/rails#43434
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rafaelfranca rafaelfranca rafaelfranca requested changes

Assignees
No one assigned
Labels
activejob
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ActiveJob::Base.set doesn't configure jobs when using perform_now
3 participants
@bensheldon @ghiculescu @rafaelfranca