Problem
I’m working on a feature which user can set frequency of a job. I am using Sidekiq.
My solution is when someone updates the frequency from view, I will delete all old scheduled jobs and create a new job in the action of controller.
I’m looking for a cleaner and more succinct way to do this.
Right now, I have a frequency column on the settings table. There is an update_frequency
method in DashboardController
to delete all old scheduled jobs and create a new scheduled job.
app/controllers/dashboard_controller.rb
class DashboardController < ApplicationController
def update_frequency
setting = Setting.find_by(id: params[:id])
if setting.blank?
render json: {status: 404, msg: 'Not found.'}
else
if setting.update_attributes(frequency: params[:frequency])
# find and delete all scheduled jobs
scheduler = Sidekiq::ScheduledSet.new
old_jobs = scheduler.select {|work| work.klass == 'HardWorker'}
old_jobs.each(&:delete) if old_jobs.present?
# add new schedule job
HardWorker.perform_in(setting.frequency.minutes)
render json: {status: 200, msg: 'update frequency successful.'}
else
render json: {status: 500, msg: 'update frequency failed.'}
end
end
end
A HardWorker
to do something and create a new scheduled job:
app/workers/hard_worker.rb
class HardWorker
include Sidekiq::Worker
def perform
# Do something
setting = Setting.first
HardWorker.perform_in(setting.frequency.minutes)
end
end
Solution
I don’t think there is a more succinct way to do that with Sidekiq, you will probably need to delete and recreate jobs.
Some comments
You probably don’t need to check if old_jobs.present?
when deleting. I am not familiar with ScheduledSet#select
but I would imagine that it returns an empty array when no job is selected.
You don’t need to update your jobs when the frequency is the same as the one already set. Even if you check it in the frontend it is a good idea to check it here to in case the view and the database are out of sync (read: in case someone changed the setting while your view was open).
Instead of using nested ifs, I like to use guards.
I also like to extract local variables into methods (see def setting
).
Here is an example of how you could refactor it.
It may seem longer, but it is easier to understand when looking at the update_frequency
method. If you need more precise understanding on what is considered as same_frequency
, or reschedule_jobs
, then you can dive into these methods. Otherwise looking at update_frequency
probably gives you everything you need to understand when browsing the code (in 6 months when you will have forgotten of this part)
class DashboardController < ApplicationController
def update_frequency
return render_not_found unless setting
return render_same_frequency if same_frequency?
return render_update_failed unless update_model
reschedule_jobs
render json: { status: 200, msg: 'update frequency successful.' }
end
def render_not_found
render json: { status: 404, msg: 'Not found.' }
end
def render_update_failed
render json: { status: 500, msg: 'update frequency failed.' }
end
def render_same_frequency
render json: { status: 200, msg: 'Frequency updated to the same value' }
end
def update_model
setting.update_attributes(frequency: new_frequency)
end
def same_frequency?
setting.frequency == params[:frequency].to_i
end
def setting
@setting ||= Setting.find_by(id: params[:id])
end
def reschedule_jobs
# find and delete all scheduled jobs
Sidekiq::ScheduledSet.new
.select { |work| work.klass == 'HardWorker' }
.each(&:delete)
# add new schedule job
HardWorker.perform_in(setting.frequency.minutes)
end
end