From a818d9f7adc1c90ab1afe0fb9a65396479a9a39d Mon Sep 17 00:00:00 2001 From: Alexandre Podlewski Date: Mon, 11 Dec 2023 13:40:22 +0100 Subject: [PATCH 1/8] Re-enable private sheet test --- test/repositories/drive_repository_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/repositories/drive_repository_test.rb b/test/repositories/drive_repository_test.rb index 903567f..0e8586d 100644 --- a/test/repositories/drive_repository_test.rb +++ b/test/repositories/drive_repository_test.rb @@ -16,7 +16,6 @@ class DriveRepositoryTest < TestCase end test 'download private sheets' do - skip('This test needs CI update to work') spreadsheet_id = '149EE3axc9e0knaCZuf7nTjj0vR7EyRhGgGT3MqWKpwM' sheet_ids = %w[0 877315405] files = DriveRepository.new.download_sheets_by_id(spreadsheet_id: spreadsheet_id, sheet_ids: sheet_ids) From 32bdbc4864e65232ad73dcc5897ee72e06e118d7 Mon Sep 17 00:00:00 2001 From: Alexandre Podlewski Date: Mon, 11 Dec 2023 14:59:03 +0100 Subject: [PATCH 2/8] Update CI workflow to run tests with google service account --- .github/workflows/ruby.yml | 6 ++++++ .gitignore | 1 + 2 files changed, 7 insertions(+) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 7f32427..9c337e1 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -20,5 +20,11 @@ jobs: bundle install --jobs 4 --retry 3 - name: Run linter run: bundle exec rubocop + - run: 'echo "$GOOGLE_APPLICATION_CREDENTIALS" > account_secret.json' + shell: bash + env: + GOOGLE_APPLICATION_CREDENTIALS: ${{secrets.GOOGLE_APPLICATION_CREDENTIALS}} - name: Run tests run: bundle exec rake test + env: + GOOGLE_APPLICATION_CREDENTIALS: 'account_secret.json' diff --git a/.gitignore b/.gitignore index 58b188b..c825cc7 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ ad_localize/tmp/ .ruby-version pkg/ exports/ +account_secret.json From 8d7e14398778c1de630fad617cc42d90830bd851 Mon Sep 17 00:00:00 2001 From: Alexandre Podlewski Date: Mon, 11 Dec 2023 17:03:42 +0100 Subject: [PATCH 3/8] Split tests to handle public and private google sheet access --- .github/workflows/ruby.yml | 6 ++-- Rakefile | 8 ++++- test/repositories/drive_repository_test.rb | 17 ----------- .../private_drive_repository_test.rb | 29 +++++++++++++++++++ 4 files changed, 40 insertions(+), 20 deletions(-) create mode 100644 test/repositories/private_drive_repository_test.rb diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 9c337e1..3bb2528 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -20,11 +20,13 @@ jobs: bundle install --jobs 4 --retry 3 - name: Run linter run: bundle exec rubocop + - name: Run tests + run: bundle exec rake test - run: 'echo "$GOOGLE_APPLICATION_CREDENTIALS" > account_secret.json' shell: bash env: GOOGLE_APPLICATION_CREDENTIALS: ${{secrets.GOOGLE_APPLICATION_CREDENTIALS}} - - name: Run tests - run: bundle exec rake test + - name: Run private google sheet tests + run: bundle exec rake test_private_google_sheet env: GOOGLE_APPLICATION_CREDENTIALS: 'account_secret.json' diff --git a/Rakefile b/Rakefile index 89a08dc..c2efb39 100644 --- a/Rakefile +++ b/Rakefile @@ -5,7 +5,13 @@ require "rake/testtask" Rake::TestTask.new(:test) do |t| t.libs << "test" t.libs << "lib" - t.test_files = FileList["test/**/*_test.rb"] + t.test_files = FileList["test/**/*_test.rb"].exclude("**/private_drive_repository_test.rb") +end + +Rake::TestTask.new(:test_private_google_sheet) do |t| + t.libs << "test" + t.libs << "lib" + t.test_files = FileList["test/**/private_drive_repository_test.rb"] end task :default => :test diff --git a/test/repositories/drive_repository_test.rb b/test/repositories/drive_repository_test.rb index 0e8586d..14ccbe0 100644 --- a/test/repositories/drive_repository_test.rb +++ b/test/repositories/drive_repository_test.rb @@ -15,29 +15,12 @@ class DriveRepositoryTest < TestCase end end - test 'download private sheets' do - spreadsheet_id = '149EE3axc9e0knaCZuf7nTjj0vR7EyRhGgGT3MqWKpwM' - sheet_ids = %w[0 877315405] - files = DriveRepository.new.download_sheets_by_id(spreadsheet_id: spreadsheet_id, sheet_ids: sheet_ids) - assert_equal 2, files.size - assert_not_equal files.first.read, files.second.read - files.each do |file| - assert file.size > 0 - end - end - test 'download all sheets in public spreadsheet' do spreadsheet_id = '10tmRtgFi1hG53fCcTTHgBjS5or_eCrHkwq4kJCLrYRA' files = DriveRepository.new.download_all_sheets(spreadsheet_id: spreadsheet_id) assert_empty files end - test 'download all sheets in private spreadsheet' do - spreadsheet_id = '149EE3axc9e0knaCZuf7nTjj0vR7EyRhGgGT3MqWKpwM' - files = DriveRepository.new.download_all_sheets(spreadsheet_id: spreadsheet_id) - assert_empty files - end - test 'handle error on download specific sheets' do spreadsheet_id = '10tmRtgFi1hG53fCcTTHgBjS5or_eCrHkwq4kJCLrYRA' fake_sheet_ids = %w[-2 -1] diff --git a/test/repositories/private_drive_repository_test.rb b/test/repositories/private_drive_repository_test.rb new file mode 100644 index 0000000..d8d60d3 --- /dev/null +++ b/test/repositories/private_drive_repository_test.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true +require 'test_helper' + +module AdLocalize + module Repositories + class PrivateDriveRepositoryTest < TestCase + test 'download private sheets' do + spreadsheet_id = '149EE3axc9e0knaCZuf7nTjj0vR7EyRhGgGT3MqWKpwM' + sheet_ids = %w[0 877315405] + files = DriveRepository.new.download_sheets_by_id(spreadsheet_id: spreadsheet_id, sheet_ids: sheet_ids) + assert_equal 2, files.size + assert_not_equal files.first.read, files.second.read + files.each do |file| + assert file.size > 0 + end + end + + test 'download all sheets in private spreadsheet' do + spreadsheet_id = '149EE3axc9e0knaCZuf7nTjj0vR7EyRhGgGT3MqWKpwM' + files = DriveRepository.new.download_all_sheets(spreadsheet_id: spreadsheet_id) + assert_equal 2, files.size + assert_not_equal files.first.read, files.second.read + files.each do |file| + assert file.size > 0 + end + end + end + end +end From e393d1fc2cd97338c09cfa80dff23ba8b1f54997 Mon Sep 17 00:00:00 2001 From: Alexandre Podlewski Date: Mon, 11 Dec 2023 17:05:59 +0100 Subject: [PATCH 4/8] Fix linter warnings --- .rubocop_todo.yml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 37084ef..a9ad65c 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2023-12-12 14:55:05 UTC using RuboCop version 1.58.0. +# on 2023-12-12 15:16:08 UTC using RuboCop version 1.58.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -14,7 +14,7 @@ Gemspec/OrderedDependencies: Exclude: - 'ad_localize.gemspec' -# Offense count: 86 +# Offense count: 87 # This cop supports safe autocorrection (--autocorrect). Layout/EmptyLineAfterMagicComment: Enabled: false @@ -184,7 +184,7 @@ Style/NegatedIf: Exclude: - 'lib/ad_localize/interactors/generate_strings.rb' -# Offense count: 2 +# Offense count: 3 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: EnforcedStyle, AllowedMethods, AllowedPatterns. # SupportedStyles: predicate, comparison @@ -192,6 +192,7 @@ Style/NumericPredicate: Exclude: - 'spec/**/*' - 'test/repositories/drive_repository_test.rb' + - 'test/repositories/private_drive_repository_test.rb' # Offense count: 1 # This cop supports safe autocorrection (--autocorrect). @@ -283,19 +284,20 @@ Style/StringConcatenation: - 'lib/ad_localize/serializers/strings_serializer.rb' - 'lib/ad_localize/serializers/templated_serializer.rb' -# Offense count: 142 +# Offense count: 146 # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline. # SupportedStyles: single_quotes, double_quotes Style/StringLiterals: Enabled: false -# Offense count: 3 +# Offense count: 4 # This cop supports unsafe autocorrection (--autocorrect-all). Style/ZeroLengthPredicate: Exclude: - 'lib/ad_localize/mappers/locale_wording_to_hash.rb' - 'test/repositories/drive_repository_test.rb' + - 'test/repositories/private_drive_repository_test.rb' # Offense count: 15 # This cop supports safe autocorrection (--autocorrect). From 51f203432868cd6bc597496f61e6f03fce649c7e Mon Sep 17 00:00:00 2001 From: Alexandre Podlewski Date: Mon, 11 Dec 2023 17:35:20 +0100 Subject: [PATCH 5/8] Prevent rate limit errors by limiting execution of authenticated tests --- .github/workflows/ruby.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 3bb2528..7853dd0 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -27,6 +27,7 @@ jobs: env: GOOGLE_APPLICATION_CREDENTIALS: ${{secrets.GOOGLE_APPLICATION_CREDENTIALS}} - name: Run private google sheet tests + if: matrix.ruby == '3.2.0' # Limit execution on latest ruby version to prevent reaching rate limits run: bundle exec rake test_private_google_sheet env: GOOGLE_APPLICATION_CREDENTIALS: 'account_secret.json' From 1072e6f7a1f983a9a7c0548580820b4aeb936da8 Mon Sep 17 00:00:00 2001 From: Alexandre Podlewski Date: Mon, 11 Dec 2023 18:18:34 +0100 Subject: [PATCH 6/8] Increase retries when getting google sheet files to prevent rate limit exceptions --- lib/ad_localize/repositories/drive_repository.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ad_localize/repositories/drive_repository.rb b/lib/ad_localize/repositories/drive_repository.rb index 7d62034..7a77b10 100644 --- a/lib/ad_localize/repositories/drive_repository.rb +++ b/lib/ad_localize/repositories/drive_repository.rb @@ -17,7 +17,7 @@ def download_sheets_by_id(spreadsheet_id:, sheet_ids:) sheet_ids.filter_map do |sheet_id| begin url = export_url(spreadsheet_id: spreadsheet_id, sheet_id: sheet_id) - string = @drive_service.http(:get, url, options: { retries: 3, max_elapsed_time: 60 }) + string = @drive_service.http(:get, url, options: { retries: 5, max_elapsed_time: 120 }) next unless string tempfile = Tempfile.new From ef7763d7f19e810fbef2a69e5bd677db7274a6e8 Mon Sep 17 00:00:00 2001 From: Alexandre Podlewski Date: Tue, 12 Dec 2023 16:33:23 +0100 Subject: [PATCH 7/8] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b3f341..6e4c374 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Updated dependencies +- Increase number of retries to 5 and timeout dalay to 120s when accessing Google APIs. This reduce the amount of rate limit error when using private Google spreadsheet. ## 6.1.0 ### Added From a1109033b0f7a30493f80cd3e77c3692537df162 Mon Sep 17 00:00:00 2001 From: Edouard Siegel Date: Thu, 14 Dec 2023 15:47:37 +0100 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e4c374..a720e66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Updated dependencies -- Increase number of retries to 5 and timeout dalay to 120s when accessing Google APIs. This reduce the amount of rate limit error when using private Google spreadsheet. +- Increase number of retries to 5 and timeout delay to 120s when accessing Google APIs. This reduces the amount of rate limit error when using private Google spreadsheet. ## 6.1.0 ### Added