From 757d77f01136a44bda3e4edb023107b208a32d27 Mon Sep 17 00:00:00 2001 From: "apurvis@lumoslabs.com" Date: Tue, 14 Nov 2017 19:35:51 -0500 Subject: [PATCH 1/5] Swap out pester for Retriable; Retry github errors --- Gemfile | 3 ++- Gemfile.lock | 9 +++++---- config/initializers/pester.rb | 20 ++++++++------------ lib/csv_helper/aws.rb | 16 +++++++++------- lib/github.rb | 2 +- lib/schemas/descriptor.rb | 8 ++------ 6 files changed, 27 insertions(+), 31 deletions(-) diff --git a/Gemfile b/Gemfile index cee48b8..e39130c 100644 --- a/Gemfile +++ b/Gemfile @@ -21,10 +21,10 @@ gem 'foreman' gem 'haml' gem 'hyperresource' gem 'jquery-rails' +gem 'json', '>= 1.8.5' gem 'omniauth-google-oauth2' gem 'omniauth-saml', '~> 1.4.1' gem 'paper_trail', '~> 4.0.0' -gem 'pester', '~> 1.0' gem 'pg' gem 'pg_stream', '~> 0.1.0' gem 'puma', '2.11.3' @@ -34,6 +34,7 @@ gem 'responders', '~> 2.0' gem 'resque' gem 'resque-pool', '~> 0.5.0' gem 'resque-web', '0.0.6', require: 'resque_web' +gem 'retriable', '~> 3.1' gem 'roar' gem 'rollbar', '~> 2.3.0' gem 'sass-rails' diff --git a/Gemfile.lock b/Gemfile.lock index e691c61..bfb2024 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -161,7 +161,7 @@ GEM thor (>= 0.14, < 2.0) jquery-ui-rails (5.0.5) railties (>= 3.2.16) - json (1.8.3) + json (1.8.6) jwt (1.5.1) kaminari (0.16.3) actionpack (>= 3.0.0) @@ -214,7 +214,6 @@ GEM activerecord (>= 3.0, < 6.0) activesupport (>= 3.0, < 6.0) request_store (~> 1.1) - pester (1.0.0) pg (0.18.3) pg_stream (0.1.0) pg (>= 0.18.2) @@ -317,6 +316,7 @@ GEM http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 3.0) netrc (~> 0.7) + retriable (3.1.1) roar (1.0.3) representable (>= 2.0.1, <= 3.0.0) rollbar (2.3.0) @@ -450,11 +450,11 @@ DEPENDENCIES haml hyperresource jquery-rails + json (>= 1.8.5) launchy omniauth-google-oauth2 omniauth-saml (~> 1.4.1) paper_trail (~> 4.0.0) - pester (~> 1.0) pg pg_stream (~> 0.1.0) poltergeist @@ -470,6 +470,7 @@ DEPENDENCIES resque resque-pool (~> 0.5.0) resque-web (= 0.0.6) + retriable (~> 3.1) roar rollbar (~> 2.3.0) rspec-rails @@ -486,4 +487,4 @@ DEPENDENCIES zeroclipboard-rails (= 0.1.0) BUNDLED WITH - 1.11.2 + 1.15.1 diff --git a/config/initializers/pester.rb b/config/initializers/pester.rb index 924ab59..a73b66b 100644 --- a/config/initializers/pester.rb +++ b/config/initializers/pester.rb @@ -1,16 +1,12 @@ -Pester.configure do |c| - c.logger = Rails.logger - - c.environments[:schema_refresh] = { - delay_interval: 3, - max_attempts: 15, - on_retry: Pester::Behaviors::Sleep::Linear +Retriable.configure do |c| + c.contexts[:schema_refresh] = { + base_interval: 3, + tries: 15, } - c.environments[:s3] = { - delay_interval: 30.seconds, - max_attempts: 10, - on_retry: Pester::Behaviors::Sleep::Constant, - reraise_error_classes: [Aws::S3::Errors::NoSuchKey, Aws::S3::Errors::NoSuchBucket] + c.contexts[:s3] = { + base_interval: 30, + multiplier: 1, + tries: 10 } end diff --git a/lib/csv_helper/aws.rb b/lib/csv_helper/aws.rb index a7e55ef..c14fab6 100644 --- a/lib/csv_helper/aws.rb +++ b/lib/csv_helper/aws.rb @@ -4,18 +4,20 @@ class Aws < Base S3_FOLDER = APP_CONFIG['s3_folder'] || 'results' def url - Pester.s3.retry do + Retriable.with_context(:s3) do obj = AwsS3.resource.bucket(S3_BUCKET).object(key) - if obj.exists? - obj.presigned_url(:get, response_content_disposition: "attachment; filename=result_#{@result_id}.csv", expires_in: 3600) - else - nil - end + return nil unless obj.exists? + + obj.presigned_url( + :get, + response_content_disposition: "attachment; filename=result_#{@result_id}.csv", + expires_in: 3600 + ) end end def store! - Pester.s3.retry do + Retriable.with_context(:s3) do obj = AwsS3.resource.bucket(S3_BUCKET).object(key) obj.upload_file(filepath) end diff --git a/lib/github.rb b/lib/github.rb index fdcf10d..6ac3fcf 100644 --- a/lib/github.rb +++ b/lib/github.rb @@ -43,7 +43,7 @@ def self.send_request(verb, url, params={}) request.content_type = 'application/json' end - http.request(request) + Retriable.retriable { http.request(request) } end def self.get(url, params={}) diff --git a/lib/schemas/descriptor.rb b/lib/schemas/descriptor.rb index 9cd3201..9709583 100644 --- a/lib/schemas/descriptor.rb +++ b/lib/schemas/descriptor.rb @@ -64,12 +64,8 @@ def schema_refresher def refresh_schema Rails.logger.info('Schemas::Descriptor.refresh_schema') - result = nil - Pester.schema_refresh.retry do - result = exec_schema_query - end - if result + if (result = Retriable.with_context(:schema_refresh) { exec_schema_query }) redis_store!(filter_tables(result.to_a)) @cache = redis_retrieve end @@ -84,7 +80,7 @@ def exec_schema_query def filter_tables(schemas) return schemas unless TABLE_BLACKLIST - + schemas.reject do |column| schema_blacklist = TABLE_BLACKLIST[column['table_schema']] next unless schema_blacklist From 72a9aad805ec9364fe564bf88067ef537305ba33 Mon Sep 17 00:00:00 2001 From: "apurvis@lumoslabs.com" Date: Tue, 14 Nov 2017 19:38:21 -0500 Subject: [PATCH 2/5] Update travis config --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 572414d..d1b7b1f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,10 @@ cache: - bundler - apt rvm: - - ruby-2.1.6 + - 2.1.6 + - 2.2.8 + - 2.3.5 + - 2.4.2 addons: postgresql: "9.2" From c33a6e770259be3edf3bd86e8ebd258a2415b61f Mon Sep 17 00:00:00 2001 From: "apurvis@lumoslabs.com" Date: Tue, 14 Nov 2017 19:47:58 -0500 Subject: [PATCH 3/5] Travis config --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index d1b7b1f..9a57282 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,6 +31,8 @@ env: matrix: - TEST_SUITE="rspec spec --color --format documentation" - TEST_SUITE="rake karma:run" + allow_failures: + - rvm: 2.4.2 before_install: - "export DISPLAY=:99.0" From 10bc362bae2f5c11b2d53ab89ec0680d913b805c Mon Sep 17 00:00:00 2001 From: "apurvis@lumoslabs.com" Date: Tue, 14 Nov 2017 19:49:07 -0500 Subject: [PATCH 4/5] Travis config --- .travis.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9a57282..63aa9f0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,8 +31,10 @@ env: matrix: - TEST_SUITE="rspec spec --color --format documentation" - TEST_SUITE="rake karma:run" - allow_failures: - - rvm: 2.4.2 + +matrix: + allow_failures: + - rvm: 2.4.2 before_install: - "export DISPLAY=:99.0" From c42e60b2ca4b3fc9d5348f8f26a8bff84af65bef Mon Sep 17 00:00:00 2001 From: "apurvis@lumoslabs.com" Date: Wed, 15 Nov 2017 02:46:08 -0500 Subject: [PATCH 5/5] Use an on_retry Proc to always raise certain S3 exceptions --- config/initializers/pester.rb | 12 ------------ config/initializers/retriable.rb | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 12 deletions(-) delete mode 100644 config/initializers/pester.rb create mode 100644 config/initializers/retriable.rb diff --git a/config/initializers/pester.rb b/config/initializers/pester.rb deleted file mode 100644 index a73b66b..0000000 --- a/config/initializers/pester.rb +++ /dev/null @@ -1,12 +0,0 @@ -Retriable.configure do |c| - c.contexts[:schema_refresh] = { - base_interval: 3, - tries: 15, - } - - c.contexts[:s3] = { - base_interval: 30, - multiplier: 1, - tries: 10 - } -end diff --git a/config/initializers/retriable.rb b/config/initializers/retriable.rb new file mode 100644 index 0000000..76a49e8 --- /dev/null +++ b/config/initializers/retriable.rb @@ -0,0 +1,16 @@ +Retriable.configure do |c| + c.contexts[:schema_refresh] = { + base_interval: 3, + tries: 15 + } + + c.contexts[:s3] = { + base_interval: 30, + multiplier: 1, + tries: 10, + on_retry: Proc.new do |exception| + # Don't allow retries of misconfigured AWS key errors + raise exception if [Aws::S3::Errors::NoSuchKey, Aws::S3::Errors::NoSuchBucket].include?(exception.class) + end + } +end