Allow subclasses to inherit more resque-retry configuration

[branliu0]

At work, we are using a ResqueBase class that all of our Resque jobs derive from. It would be great to be able to configure some resque-retry information there, so that we can have "defaults" that all of our resque jobs automatically inherit, but can override if desired.

Either (A) we automatically inherit more resque-retry options (but I think this is a bit of a breaking change). Or (B) we add the ability for the user to specify which options should be inherited. e.g.,

class InheritingJob
  extend Resque::Plugins::Retry

  @retry_exceptions = [MyException]
  @inherited_options = [:retry_exceptions]
end

What do you think? If it sounds reasonable the implementation is pretty straightforward.

[lantins]

Some information should already be passed on through class inheritance.

Can you please explain what the problem is with the existing support?

[branliu0]

I feel a bit confused on how the inheritance should work... but since the config variables are defined as class instance variables, it doesn't seem like they are getting inherited?

In particular, this doesn't work for @retry_limit, and I'm pretty sure it doesn't work for other variables too.

[5] pry(main)> MyJob.retry_limit
=> 1
[6] pry(main)> ResqueBase.retry_limit
=> 10
[9] pry(main)> ResqueBase.instance_variable_get('@retry_limit')
=> 10
[10] pry(main)> MyJob.instance_variable_get('@retry_limit')
=> 1

[jzaleski]

We copy a number of class instance-variables to the receiver in the inherited method. Should we maybe just copy these as well? I think @lantins is probably the best resource on this, but I'll gladly help wherever I can.

[lantins]

I think resque-retry lib should leave the responsibility to the developer.

You could use the inherited method to copy over more instance vars, or you could do something like this:

module JobRetryDefaults
  def retry_limit
    @retry_limit ||= 64
  end

  def retry_exceptions
    @retry_exceptions ||= [RuntimeError, Exception]
  end
end

class JobExample
  extend Resque::Plugins::Retry
  extend JobRetryDefaults

  @queue = :somequeue
  # override defaults
  @retry_exceptions = [Timeout::Error]

  def self.perform(*args)
    # do work
  end
end

[jzaleski]

It seems like right now we support both. Perhaps we should just buff up the inherited method to copy over all of the instance vars we use in resque-retry however (for consistency sake if nothing else)? On Nov 3, 2015 7:33 PM, "Luke Antins" [email protected] wrote:

I think resque-retry lib should leave the responsibility to the developer.

You could use the inherited method to copy over more instance vars, or you could do something like this:

module JobRetryDefaultsModule def retry_limit @retry_limit ||= 64 end

def retry_exceptions @retry_exceptions ||= [RuntimeError, Exception] endend class JobExample extend Resque::Plugins::Retry extend JobRetryDefaultsModule

@queue = :somequeue

override defaults

@retry_exceptions = [Timeout::Error]

def self.perform(*args) # do work endend

— Reply to this email directly or view it on GitHub https://github.com/lantins/resque-retry/issues/129#issuecomment-153533797 .

[lantins]

Should we be copying any instance vars at all?

It seems a bit 'magic' to me, different to what happens with vanilla Ruby.

Resque does not copy over the @queue instance var and have stated they want this type of configuration to be explicit, so they wont allow it to be copied automatically. Although this is an old reference: https://github.com/resque/resque/issues/32

It is possible to implement this type 'job defaults' yourself, without a lot of code.

If we copied over all instance vars

1. Is everything configured by instance vars? do we need to copy anything else over?
2. Would there be any negative effects by doing this?

[jzaleski]

My vote would be for all or none. If resque-retry can function [properly] w/o the inherited method I would love to remove it.

[lantins]

My gut tells me we don't need the inherited method for the lib to work correctly. If that holds, I'd also like to see it removed.

We can always add a short example of how you can configure job defaults.

[NobodysNightmare]

Hey, just wanted to let you know that I actually fell into that pit today. I assumed my base class instance variables would be picked up in subclasses, but actually they are not.

So you would propose to overwrite the corresponding methods (e.g. retry_exceptions) in my base class like the following, right?

class MyBaseClass
  extend Resque::Plugins::ExponentialBackoff

  class << self
    def retry_exceptions
      # subclass could still override this method  or set the instance variable
      @retry_exceptions ||= [DefaultRetryException]
    end
  end
end