Configure Error::RequestLimitExceeded backoff in ClientWrap

We've been running into a few rate limit exceeded issues with some of our tests, and I starting poking around and noticed there was a PR almost 2 years ago that added a backoff and retry mechanism, which is awesome! Looking at the code that's there, the attributes to configure the backoff timing have some defaults and seem to provide a mechanism to configure them:

awspec/lib/awspec/helper/client_wrap.rb

Lines 7 to 11 in d73417b

    
           @backoff       = args.key?(:backoff) ? args[:backoff] : 0.0 
        
           @orig_backoff  = @backoff 
        
           @iteration     = args.key?(:iteration) ? args[:iteration] : 1 
        
           @orig_iter     = @iteration 
        
           @backoff_limit = args.key?(:backoff_limit) ? args[:backoff_limit] : 30.0

Looking at the only place where ClientWrap is instantiated, it takes it CLIENT_OPTIONS as an argument:

awspec/lib/awspec/helper/finder.rb

Line 159 in e61fbad

Awspec::Helper::ClientWrap.new(client.new(CLIENT_OPTIONS))

But CLIENT_OPTIONS is only allowing http_proxy/https_proxy to be configured:

awspec/lib/awspec/helper/finder.rb

Lines 150 to 152 in e61fbad

    
           CLIENT_OPTIONS = { 
        
             http_proxy: ENV['http_proxy'] || ENV['https_proxy'] || nil 
        
           }

Is there any other mechanism to configure ClientWrap that I'm missing? Or is it hard coded?

Would love to send a PR but want to check before doing so, thanks again for an amazing tool!

Hi @amaltson . Thank you for your report!

Is there any other mechanism to configure ClientWrap that I'm missing? Or is it hard coded?

is it hard coded.

Awspec::Helper::ClientWrap allow args to configure.

awspec/lib/awspec/helper/client_wrap.rb

Line 4 in d73417b

def initialize(real_client = nil, args = {})

but not used.

Would love to send a PR but want to check before doing so, thanks again for an amazing tool!

Thank you !

@k1LoW I finally have a bit of time to work on this (happy Hacktoberfest haha), and wanted to run the design by you a bit.

I'm thinking that in order to allow configuration of the ClientWrapper, I don't want to introduce new environment variables in the CLIENT_OPTIONS configuration, that doesn't feel very "RSpec" like

awspec/lib/awspec/helper/finder.rb

Lines 150 to 152 in e61fbad

    
           CLIENT_OPTIONS = { 
        
             http_proxy: ENV['http_proxy'] || ENV['https_proxy'] || nil 
        
           }

Instead, I'm thinking to introduce something like RSpec's configuration block and allow it to be accessible in the same way. In order to make it a top level change, I'll add a new config folder and overload the Awspec module there adding a new configure method that takes in a block that we'll instance_eval.

From a customer's prospective it'll look something like this:

# In the `spec_helper.rb`
require 'awspec'

Awspec.configure do |config|
  config.client_iteration 5
  config.client_backoff_limit 60
end

And within the finder.rb we'd read off that configuration and pass it to the ClientWrapper. What do you think?

@amaltson

From a customer's prospective it'll look something like this:

GREAT !!! I like it !!

🎉 glad I checked with you, thanks! I’ll send over a PR in the next couple days.

	@backoff = args.key?(:backoff) ? args[:backoff] : 0.0
	@orig_backoff = @backoff
	@iteration = args.key?(:iteration) ? args[:iteration] : 1
	@orig_iter = @iteration
	@backoff_limit = args.key?(:backoff_limit) ? args[:backoff_limit] : 30.0

	CLIENT_OPTIONS = {
	http_proxy: ENV['http_proxy'] \|\| ENV['https_proxy'] \|\| nil
	}