0
\$\begingroup\$

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
\$\endgroup\$
0
\$\begingroup\$

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
\$\endgroup\$
  • \$\begingroup\$ You're welcome! Don't hesitate to mark this answer as accepted then @andrewyang \$\endgroup\$ – Guillaume Mar 26 '18 at 14:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.