Merge pull request #11 from lumoslabs/self_refactor

Refactor

.rubocop.yml

@@ -1,14 +1,29 @@
 Metrics/AbcSize:
-  Max: 20
+  Max: 27
+
+Metrics/CyclomaticComplexity:
+  Max: 7
 
 Metrics/LineLength:
   Max: 140
 
+Metrics/MethodLength:
+  Max: 23
+
+Metrics/PerceivedComplexity:
+  Max: 9
+
 Style/Documentation:
-    Enabled: false
+  Enabled: false
+
+Style/ModuleFunction:
+  Enabled: false
 
 Style/RaiseArgs:
-    Enabled: false
+  Enabled: false
+
+Style/SpaceAroundOperators:
+  Enabled: false
 
 Style/SignalException:
   Enabled: false

README.md

@@ -11,7 +11,7 @@ From the outset, the goal of Pester is to offer a simple interface. For example:
 
     irb(main):001:0> require 'pester'
     => true
-    irb(main):002:0> Pester.retry { fail 'derp' }
+    irb(main):002:0> Pester.retry_constant { fail 'derp' }
     W, [2015-04-04T10:37:46.413158 #87600]  WARN -- : Failure encountered: derp, backing off and trying again 3 more times. etc etc
 
 will retry the block--which always fails--until Pester has exhausted its amount of retries. With no options provided, this will sleep for a constant number of seconds between attempts.
@@ -24,7 +24,7 @@ Pester's basic retry behaviors are defined by three options:
 
 `delay_interval` is the unit, in seconds, that will be delayed between attempts. Normally, this is just the total number of seconds, but it can change with other `Behavior`s. `max_attempts` is the number of tries Pester will make, including the initial one. If this is set to 1, Pester will basically not retry; less than 1, it will not even bother executing the block:
 
-    irb(main):001:0> Pester.retry(max_attempts: 0) { puts 'Trying...'; fail 'derp' }
+    irb(main):001:0> Pester.retry_constant(max_attempts: 0) { puts 'Trying...'; fail 'derp' }
     => nil
 
 `on_retry` defines the behavior between retries, which can either be a custom block of code, or one of the predefined `Behavior`s, specifically in `Pester::Behaviors::Sleep`. If passed an empty lambda/block, Pester will immediately retry. When writing a custom behavior, `on_retry` expects a block that can be called with two parameters, `attempt_num`, and `delay_interval`, the idea being that these will mostly be used to define a function that determines just how long to sleep between attempts.
@@ -35,7 +35,7 @@ Three behaviors are provided out-of-the box:
 * `Linear` simply multiplies `attempt_num` by `delay_interval` and sleeps for that many seconds
 * `Exponential` sleeps for 2<sup>(`attempt_num` - 1)</sup> * `delay_interval` seconds
 
-All three are available either by passing the behaviors to `on_retry`, or by calling the increasingly-verbosely-named `retry` (constant), `retry_with_backoff` (linear), or `retry_with_exponential_backoff` (exponential).
+All three are available either by passing the behaviors to `on_retry`, or by calling the increasingly-verbosely-named `retry_constant` (constant), `retry_with_backoff` (linear), or `retry_with_exponential_backoff` (exponential). `retry` by itself *will not* actually retry anything, unless provided with an `on_retry` function, either per-call or in the relevant environment.
 
 Pester does log retry attempts (see below), however custom retry behavior that wraps existing `Behavior`s may be appropriate for logging custom information, incrementing statsd counters, etc. Also of note, different loggers can be passed per-call via the `logger` option.
 
@@ -51,7 +51,7 @@ Pester can be configured to be picky about what it chooses to retry and what it
 
 The first two are mutually-exclusive whitelist and blacklists, both taking either a single error class or an array. Raising an error not covered by `retry_error_classes` (whitelist) causes it to immediately fail:
 
-    irb(main):002:0> Pester.retry(retry_error_classes: NotImplementedError) do
+    irb(main):002:0> Pester.retry_constant(retry_error_classes: NotImplementedError) do
       puts 'Trying...'
       fail 'derp'
     end
@@ -60,7 +60,7 @@ The first two are mutually-exclusive whitelist and blacklists, both taking eithe
 
 Raising an error covered by `reraise_error_classes` (blacklist) causes it to immediately fail:
 
-    irb(main):002:0> Pester.retry(reraise_error_classes: NotImplementedError) do
+    irb(main):002:0> Pester.retry_constant(reraise_error_classes: NotImplementedError) do
       puts 'Trying...'
       raise NotImplementedError.new('derp')
     end
@@ -69,7 +69,7 @@ Raising an error covered by `reraise_error_classes` (blacklist) causes it to imm
 
 `retry_error_messages` also takes a single string or array, and calls `include?` on the error message. If it matches, the error's retried:
 
-    irb(main):002:0> Pester.retry(retry_error_messages: 'please') do
+    irb(main):002:0> Pester.retry_constant(retry_error_messages: 'please') do
       puts 'Trying...'
       fail 'retry this, please'
     end
@@ -78,7 +78,7 @@ Raising an error covered by `reraise_error_classes` (blacklist) causes it to imm
 
 Because it calls `include?`, this also works for regexes:
 
-    irb(main):002:0> Pester.retry(retry_error_messages: /\d/) do
+    irb(main):002:0> Pester.retry_constant(retry_error_messages: /\d/) do
       puts 'Trying...'
       fail 'retry this 2'
     end
@@ -92,8 +92,8 @@ Because it calls `include?`, this also works for regexes:
 The easiest way to coordinate sets of Pester options across an app is via environments--these are basically option hashes configured in Pester by name:
 
     Pester.configure do |c|
-      c.environments[:aws]      = { max_attempts: 3, delay_interval: 5 }
-      c.environments[:internal] = { max_attempts: 2, delay_interval: 0 }
+      c.environments[:aws]      = { max_attempts: 3, delay_interval: 5, on_retry: Pester::Behaviors::Sleep::Constant }
+      c.environments[:internal] = { max_attempts: 2, delay_interval: 0, on_retry: Pester::Behaviors::Sleep::Constant }
     end
 
 This will create two environments, `aws` and `internal`, which allow you to employ different backoff strategies, depending on the usage context. These are employed simply by calling `Pester.environment_name.retry` (where `retry` can also be another helper method):
@@ -112,7 +112,7 @@ This will create two environments, `aws` and `internal`, which allow you to empl
 
 Environments can also be merged with retry helper methods:
 
-    Pester.aws.retry  # acts different from
+    Pester.aws.retry_constant  # acts different from
     Pester.aws.retry_with_exponential_backoff
 
 where the helper method's `Behavior` will take precedence.
@@ -127,7 +127,7 @@ Pester will write retry and exhaustion information into your logs, by default us
 
 And thus:
 
-    irb(main):002:0> Pester.retry(delay_interval: 1) { puts 'Trying...'; fail 'derp' }
+    irb(main):002:0> Pester.retry_constant(delay_interval: 1) { puts 'Trying...'; fail 'derp' }
     Trying...
     Trying...
     Trying...

lib/pester.rb

@@ -5,22 +5,27 @@
 require 'pester/version'
 
 module Pester
-  def self.configure(&block)
+  extend self
+  attr_accessor :environments
+
+  def configure(&block)
     Config.configure(&block)
-    unless Config.environments.nil?
-      self.environments = Hash[Config.environments.select { |_, e| e.is_a?(Hash) }.map { |k, e| [k.to_sym, Environment.new(e)] }]
-    end
+
+    return if Config.environments.nil?
+
+    valid_environments = Config.environments.select { |_, e| e.is_a?(Hash) }
+    @environments = Hash[valid_environments.map { |k, e| [k.to_sym, Environment.new(e)] }]
   end
 
-  def self.retry(options = {}, &block)
+  def retry_constant(options = {}, &block)
     retry_action(options.merge(on_retry: Behaviors::Sleep::Constant), &block)
   end
 
-  def self.retry_with_backoff(options = {}, &block)
+  def retry_with_backoff(options = {}, &block)
     retry_action(options.merge(on_retry: Behaviors::Sleep::Linear), &block)
   end
 
-  def self.retry_with_exponential_backoff(options = {}, &block)
+  def retry_with_exponential_backoff(options = {}, &block)
     retry_action({ delay_interval: 1 }.merge(options).merge(on_retry: Behaviors::Sleep::Exponential), &block)
   end
 
@@ -49,7 +54,7 @@ def self.retry_with_exponential_backoff(options = {}, &block)
   #     FileUtils.rm_r(directory)
   #   end
 
-  def self.retry_action(opts = {}, &block)
+  def retry_action(opts = {}, &block)
     merge_defaults(opts)
     if opts[:retry_error_classes] && opts[:reraise_error_classes]
       fail 'You can only have one of retry_error_classes or reraise_error_classes'
@@ -79,25 +84,21 @@ def self.retry_action(opts = {}, &block)
     nil
   end
 
-  def respond_to?(method_sym)
+  def respond_to?(method_sym, options = {}, &block)
     super || Config.environments.key?(method_sym)
   end
 
-  def method_missing(method_sym)
-    if Config.environments.key?(method_sym)
-      Config.environments[method_sym]
+  def method_missing(method_sym, options = {}, &block)
+    if @environments.key?(method_sym)
+      @environments[method_sym]
     else
       super
     end
   end
 
-  class << self
-    attr_accessor :environments
-  end
-
   private
 
-  def self.should_retry?(e, opts = {})
+  def should_retry?(e, opts = {})
     retry_error_classes   = opts[:retry_error_classes]
     retry_error_messages  = opts[:retry_error_messages]
     reraise_error_classes = opts[:reraise_error_classes]
@@ -117,7 +118,7 @@ def self.should_retry?(e, opts = {})
     end
   end
 
-  def self.merge_defaults(opts)
+  def merge_defaults(opts)
     opts[:retry_error_classes]      = opts[:retry_error_classes] ? Array(opts[:retry_error_classes]) : nil
     opts[:retry_error_messages]     = opts[:retry_error_messages] ? Array(opts[:retry_error_messages]) : nil
     opts[:reraise_error_classes]    = opts[:reraise_error_classes] ? Array(opts[:reraise_error_classes]) : nil
@@ -127,4 +128,6 @@ def self.merge_defaults(opts)
     opts[:on_max_attempts_exceeded] ||= Behaviors::WarnAndReraise
     opts[:logger]                   ||= Config.logger
   end
+
+  alias_method :retry, :retry_action
 end

lib/pester/version.rb

@@ -1,3 +1,3 @@
 module Pester
-  VERSION = '0.2.0'
+  VERSION = '1.0.0'
 end

pester.gemspec

@@ -10,9 +10,9 @@ Gem::Specification.new do |spec|
   spec.email         = ['[email protected]']
   spec.summary       = 'Common block-based retry for external calls.'
   spec.description   = <<-EOD
-                       |We found ourselves constantly wrapping network-facing calls with all kinds of bespoke,
-                       | copied, and rewritten retry logic. This gem is an attempt to unify common behaviors,
-                       | like simple retry, retry with linear backoff, and retry with exponential backoff.
+                       We found ourselves constantly wrapping network-facing calls with all kinds of bespoke,
+                       copied, and rewritten retry logic. This gem is an attempt to unify common behaviors,
+                       like simple retry, retry with linear backoff, and retry with exponential backoff.
 EOD
   spec.homepage      = 'https://github.com/lumoslabs/pester'
   spec.license       = 'MIT'

spec/pester_spec.rb

@@ -275,22 +275,25 @@ class UnmatchedError < RuntimeError; end
     let(:environment_name) { :abc }
     let(:options) { { option: 1234 } }
 
-    it 'adds it to the Pester environment list' do
+    before do
       Pester.configure do |config|
         config.environments[environment_name] = options
       end
+    end
 
+    it 'adds it to the Pester environment list' do
       expect(Pester.environments.count).to eq(1)
     end
 
     it 'contains an Environment with the appropriate options' do
-      Pester.configure do |config|
-        config.environments[environment_name] = options
-      end
-
       expect(Pester.environments[environment_name].class).to eq(Pester::Environment)
       expect(Pester.environments[environment_name].options).to eq(options)
     end
+
+    it 'contains an Environment addressable directly from Pester with the appropriate options' do
+      expect(Pester.send(environment_name).class).to eq(Pester::Environment)
+      expect(Pester.send(environment_name).options).to eq(options)
+    end
   end
 end