From d2f14cd601af19be048041eef07752f7b8d43a6a Mon Sep 17 00:00:00 2001 From: Phil Darnowsky Date: Tue, 6 Apr 2010 15:59:55 -0400 Subject: [PATCH] Added on_permanent_failure hook --- .gitignore | 1 + README.textile | 14 +++++++++++++ lib/delayed/worker.rb | 6 ++++++ spec/sample_jobs.rb | 9 +++++++- spec/worker_spec.rb | 48 ++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 74 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index c111b3313..462cd4f83 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ *.gem +*.swp diff --git a/README.textile b/README.textile index 11ec0728b..3e0f12639 100644 --- a/README.textile +++ b/README.textile @@ -123,6 +123,20 @@ end Delayed::Job.enqueue NewsletterJob.new('lorem ipsum...', Customers.find(:all).collect(&:email)) +You can also add an optional on_permanent_failure method which will run if the job has failed too many times to be retried: + +
+class ParanoidNewsletterJob < NewsletterJob
+  def perform
+    emails.each { |e| NewsletterMailer.deliver_text_to_email(text, e) }
+  end    
+
+  def on_permanent_failure
+    page_sysadmin_in_the_middle_of_the_night
+  end
+end  
+
+ h2. Gory Details The library evolves around a delayed_jobs table which looks as follows: diff --git a/lib/delayed/worker.rb b/lib/delayed/worker.rb index 1309062ac..754ef5f22 100644 --- a/lib/delayed/worker.rb +++ b/lib/delayed/worker.rb @@ -127,6 +127,12 @@ def reschedule(job, time = nil) job.save! else say "* [JOB] PERMANENTLY removing #{job.name} because of #{job.attempts} consecutive failures.", Logger::INFO + + if job.payload_object.respond_to? :on_permanent_failure + say "* [JOB] Running on_permanent_failure hook" + job.payload_object.on_permanent_failure + end + self.class.destroy_failed_jobs ? job.destroy : job.update_attributes(:failed_at => Delayed::Job.db_time_now) end end diff --git a/spec/sample_jobs.rb b/spec/sample_jobs.rb index 36e3c9252..6fe8a8bb3 100644 --- a/spec/sample_jobs.rb +++ b/spec/sample_jobs.rb @@ -1,3 +1,5 @@ +require 'ruby-debug' + class SimpleJob cattr_accessor :runs; self.runs = 0 def perform; @@runs += 1; end @@ -12,10 +14,15 @@ class LongRunningJob def perform; sleep 250; end end +class OnPermanentFailureJob < SimpleJob + def on_permanent_failure + end +end + module M class ModuleJob cattr_accessor :runs; self.runs = 0 def perform; @@runs += 1; end end -end \ No newline at end of file +end diff --git a/spec/worker_spec.rb b/spec/worker_spec.rb index 27de7dd66..314a7778f 100644 --- a/spec/worker_spec.rb +++ b/spec/worker_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'ruby-debug' describe Delayed::Worker do def job_create(opts = {}) @@ -134,12 +135,52 @@ def job_create(opts = {}) before do @job = Delayed::Job.create :payload_object => SimpleJob.new end - + + share_examples_for "any failure more than Worker.max_attempts times" do + context "when the job's payload has an #on_permanent_failure hook" do + before do + @job = Delayed::Job.create :payload_object => OnPermanentFailureJob.new + @job.payload_object.should respond_to :on_permanent_failure + end + + it "should run that hook" do + @job.payload_object.should_receive :on_permanent_failure + Delayed::Worker.max_attempts.times { @worker.reschedule(@job) } + end + end + + context "when the job's payload has no #on_permanent_failure hook" do + # It's a little tricky to test this in a straightforward way, + # because putting a should_not_receive expectation on + # @job.payload_object.on_permanent_failure makes that object + # incorrectly return true to + # payload_object.respond_to? :on_permanent_failure, which is what + # reschedule uses to decide whether to call on_permanent_failure. + # So instead, we just make sure that the payload_object as it + # already stands doesn't respond_to? on_permanent_failure, then + # shove it through the iterated reschedule loop and make sure we + # don't get a NoMethodError (caused by calling that nonexistent + # on_permanent_failure method). + + before do + @job.payload_object.should_not respond_to(:on_permanent_failure) + end + + it "should not try to run that hook" do + lambda do + Delayed::Worker.max_attempts.times { @worker.reschedule(@job) } + end.should_not raise_exception(NoMethodError) + end + end + end + context "and we want to destroy jobs" do before do Delayed::Worker.destroy_failed_jobs = true end - + + it_should_behave_like "any failure more than Worker.max_attempts times" + it "should be destroyed if it failed more than Worker.max_attempts times" do @job.should_receive(:destroy) Delayed::Worker.max_attempts.times { @worker.reschedule(@job) } @@ -156,6 +197,8 @@ def job_create(opts = {}) Delayed::Worker.destroy_failed_jobs = false end + it_should_behave_like "any failure more than Worker.max_attempts times" + it "should be failed if it failed more than Worker.max_attempts times" do @job.reload.failed_at.should == nil Delayed::Worker.max_attempts.times { @worker.reschedule(@job) } @@ -166,7 +209,6 @@ def job_create(opts = {}) (Delayed::Worker.max_attempts - 1).times { @worker.reschedule(@job) } @job.reload.failed_at.should == nil end - end end end