amitree / delayed_job_recurring

Extends delayed_job to support recurring jobs
MIT License
84 stars 33 forks source link

Scheduling new task deletes any other instance of same task. #17

Closed andrewezzet closed 7 years ago

andrewezzet commented 7 years ago

I have a task defined in my models:

class ReminderTask
  include Delayed::RecurringJob

  queue :default

  def initialize(user)
    @user = user
  end

  def perform
    @user.send_reminder
  end
end

And I schedule it from a method in another model like this:

class Prescription < ApplicationRecord
  belongs_to :user

  def initiate_schedule
    self.times.each do |time|
      ReminderTask.new(self.user).schedule!(run_every: 1.day, run_at: time)
    end
  end
end

But any time a new ReminderTask is scheduled, any existing one gets deleted, so there is only ever one in the jobs database.

Is there any way to have multiple instances of the same task scheduled? Am I missing something?

Thanks, Andrew

afn commented 7 years ago

Sorry for the delayed response. That's an interesting use case and one I hadn't considered. Currently, there are two assumptions being made:

  1. That we want schedule! to be idempotent, i.e. calling it multiple times shouldn't schedule identical jobs multiple times
  2. That jobs are uniquely identified by class name

We could consider changing either of these assumptions. Either we could have schedule! accept an option that allows duplicate jobs to be scheduled, or modify the jobs method to be more selective about which jobs are considered duplicates. The latter would probably require jobs to be made into an instance method, since your use case would require uniquely identifying jobs by user as well as run-time. In addition, care needs to be taken to ensure that identical jobs with different serializations are considered identical (e.g. what if the order of elements in a hash is changed? Strict string comparison, the way it's being done now, won't hold up.)

Would you like to take a stab at this and submit a PR?

Gokul595 commented 7 years ago

@afn, Added a pull request #18 to schedule multiple jobs with same class.

afn commented 7 years ago

Thanks @Gokul595! Merged PR.