From 010074e42ccb76e3de232bea8663f97df5042be5 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 13 Sep 2023 12:44:33 +1000 Subject: [PATCH 01/22] Implement a prototype version of 429-auto-retry for iOS strings --- .../helper/ios/ios_l10n_helper.rb | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb index e7197fd8a..ab8175d3e 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb @@ -161,7 +161,16 @@ def self.generate_strings_file_from_hash(translations:, output_path:) # Typical examples include `{ status: 'current' }` or `{ status: 'review' }`. # @param [String, IO] destination The path or `IO`-like instance, where to write the downloaded file on disk. # - def self.download_glotpress_export_file(project_url:, locale:, filters:, destination:) + def self.download_glotpress_export_file( + project_url:, + locale:, + filters:, + destination:, + autoretry: true, + autoretry_count: 0, + autoretry_max: Fastlane::Helper::MetadataDownloader::MAX_AUTO_RETRY_ATTEMPTS, + autoretry_sleep: Fastlane::Helper::MetadataDownloader::AUTO_RETRY_SLEEP_TIME + ) query_params = (filters || {}).transform_keys { |k| "filters[#{k}]" }.merge(format: 'strings') uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations/?#{URI.encode_www_form(query_params)}") @@ -172,7 +181,27 @@ def self.download_glotpress_export_file(project_url:, locale:, filters:, destina IO.copy_stream(uri.open(options), destination) rescue StandardError => e UI.error "Error downloading locale `#{locale}` — #{e.message} (#{uri})" - retry if e.is_a?(OpenURI::HTTPError) && UI.confirm("Retry downloading `#{locale}`?") + if e.is_a?(OpenURI::HTTPError) + if e.io.status[0] == '429' && autoretry + if autoretry_count < autoretry_max + UI.message("Received 429 for `#{locale}`. Auto retrying in #{autoretry_sleep} seconds...") + sleep(autoretry_sleep) + return download_glotpress_export_file( + project_url: project_url, + locale: locale, + filters: filters, + destination: destination, + autoretry: autoretry, + autoretry_count: autoretry_count + 1, + autoretry_max: autoretry_max + ) + else + UI.user_error!("Abandoning `#{locale}` download after #{autoretry_max} attempts.") + end + elsif UI.confirm("Retry downloading `#{locale}`?") + retry + end + end return nil end end From 6bac21dcc316f9695f931148cf6b743fa7ced56e Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 16:35:32 +1100 Subject: [PATCH 02/22] Use Ruby 3.2.2 --- .ruby-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ruby-version b/.ruby-version index 74500cee1..be94e6f53 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.7.4 \ No newline at end of file +3.2.2 From 5d1b52af0c8b0872a42c7fc28f8eae2e7d266bbc Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 16:35:43 +1100 Subject: [PATCH 03/22] Use latest WebMock, 3.19.1 I thought this might help with an issue I'm experiencing where there the uri of a stubbed response is unexpectedly nil, but it did not. --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 9ee698136..c183b9fce 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -29,7 +29,7 @@ GEM i18n (>= 1.6, < 2) minitest (>= 5.1) tzinfo (~> 2.0) - addressable (2.8.4) + addressable (2.8.5) public_suffix (>= 2.0.2, < 6.0) algoliasearch (1.27.5) httpclient (~> 2.8, >= 2.8.3) @@ -320,7 +320,7 @@ GEM trailblazer-option (>= 0.1.1, < 0.2.0) uber (< 0.2.0) retriable (3.1.2) - rexml (3.2.5) + rexml (3.2.6) rmagick (4.3.0) rouge (2.0.7) rspec (3.12.0) @@ -392,7 +392,7 @@ GEM unf_ext unf_ext (0.0.8.2) unicode-display_width (2.4.2) - webmock (3.18.1) + webmock (3.19.1) addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) From f0881319b2d3a8598e579f1737af6295208b4e6a Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 16:36:48 +1100 Subject: [PATCH 04/22] Add WebMock monkey-patch to work around nil uri in response See https://github.com/bblimke/webmock/issues/469 --- spec/spec_helper.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 80cff9bfc..25cc6ae15 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -103,3 +103,14 @@ def with_tmp_file(named: nil, content: '') EMPTY_FIREBASE_TEST_LOG_PATH = File.join(__dir__, 'test-data', 'empty.json') PASSED_FIREBASE_TEST_LOG_PATH = File.join(__dir__, 'test-data', 'firebase', 'firebase-test-lab-run-passed.log') FAILED_FIREBASE_TEST_LOG_PATH = File.join(__dir__, 'test-data', 'firebase', 'firebase-test-lab-run-failure.log') + +# Monkey-patch WebMock to work around bug where response.uri is not set +# See https://github.com/bblimke/webmock/issues/469 +WebMock::HttpLibAdapters::NetHttpAdapter.instance_variable_get(:@webMockNetHTTP).class_eval do + old_request = instance_method :request + define_method :request do |request, &block| + old_request.bind(self).call(request, &block).tap do |response| + response.uri = request.uri + end + end +end From 6326cf7227c0cc94b476f96532853ca741235050 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 16:37:23 +1100 Subject: [PATCH 05/22] Add test for current metadata_download_helper behavior --- spec/metadata_download_helper_spec.rb | 42 +++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 spec/metadata_download_helper_spec.rb diff --git a/spec/metadata_download_helper_spec.rb b/spec/metadata_download_helper_spec.rb new file mode 100644 index 000000000..ee6b04bdd --- /dev/null +++ b/spec/metadata_download_helper_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe Fastlane::Helper::MetadataDownloader do + describe 'downloading from GlotPress' do + context 'when GlotPress returs a 429 code' do + it 'automatically retries' do + in_tmp_dir do |tmp_dir| + metadata_downaloder = described_class.new( + tmp_dir, + { key: { desc: 'target-file-name.txt' } }, + true + ) + + fake_url = 'https://test.com' + + count = 0 + stub_request(:get, fake_url).to_return do + count += 1 + if count == 1 + { status: 429, body: 'Too Many Requests' } + else + { status: 200, body: 'OK' } + end + end + + expect(Fastlane::UI).to receive(:message) + .with(/Received 429 for `en-AU`. Auto retrying in 20 seconds.../) + + expect(Fastlane::UI).to receive(:message) + .with(/No translation available for en-AU/) + + expect(Fastlane::UI).to receive(:success) + .with(/Successfully downloaded `en-AU`./) + + metadata_downaloder.download('en-AU', fake_url, false) + + expect(count).to eq(2) + end + end + end + end +end From ee2bb8b8682f4be645f2a9b4acb184315842583a Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 17:01:51 +1100 Subject: [PATCH 06/22] Extract retry logic in `GlotPressDownloader` helper --- .../helper/metadata_download_helper.rb | 48 ++++++++++++++++++- spec/metadata_download_helper_spec.rb | 2 +- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index 9745306d1..696d3ca06 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -1,6 +1,50 @@ require 'net/http' require 'json' +module Fastlane + module Helper + class GlotPressDownloader + AUTO_RETRY_SLEEP_TIME = 20 + MAX_AUTO_RETRY_ATTEMPTS = 30 + + def initialize(auto_retry) + @auto_retry = auto_retry + @auto_retry_attempt_counter = 0 + end + + def download(target_locale, glotpress_url, is_source) + uri = URI(glotpress_url) + response = Net::HTTP.get_response(uri) + + case response.code + when '200' + # All good pass the result along + response + when '301' + # Follow the redirect + UI.message("Received 301 for `#{locale}`. Following redirect...") + download(locale, response.header['location'], is_source) + when '429' + # We got rate-limited, auto_retry or offer to try again with a prompt + if @auto_retry && @auto_retry_attempt_counter <= MAX_AUTO_RETRY_ATTEMPTS + UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{AUTO_RETRY_SLEEP_TIME} seconds...") + sleep(AUTO_RETRY_SLEEP_TIME) + @auto_retry_attempt_counter += 1 + download(target_locale, response.uri, is_source) + elsif UI.confirm("Retry downloading `#{response.uri}` after receiving 429 from the API?") + download(target_locale, response.uri, is_source) + else + UI.error("Abandoning `#{response.uri}` download as requested.") + end + else + message = "Received unexpected #{response.code} from request to URI #{response.uri}." + UI.abort_with_message!(message) unless UI.confirm("#{message} Continue anyway?") + end + end + end + end +end + module Fastlane module Helper class MetadataDownloader @@ -19,8 +63,8 @@ def initialize(target_folder, target_files, auto_retry) # Downloads data from GlotPress, in JSON format def download(target_locale, glotpress_url, is_source) - uri = URI(glotpress_url) - response = Net::HTTP.get_response(uri) + downloader = GlotPressDownloader.new(@auto_retry) + response = downloader.download(target_locale, glotpress_url, is_source) handle_glotpress_download(response: response, locale: target_locale, is_source: is_source) end diff --git a/spec/metadata_download_helper_spec.rb b/spec/metadata_download_helper_spec.rb index ee6b04bdd..ee7e71989 100644 --- a/spec/metadata_download_helper_spec.rb +++ b/spec/metadata_download_helper_spec.rb @@ -24,7 +24,7 @@ end expect(Fastlane::UI).to receive(:message) - .with(/Received 429 for `en-AU`. Auto retrying in 20 seconds.../) + .with(/Received 429 for `#{fake_url}`. Auto retrying in 20 seconds.../) expect(Fastlane::UI).to receive(:message) .with(/No translation available for en-AU/) From fe9a93bc15a06b54e9614ed78167a9e6d328a52a Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 17:05:04 +1100 Subject: [PATCH 07/22] Remove useless parameters from extracted `GlotPressDownloader` --- .../helper/metadata_download_helper.rb | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index 696d3ca06..e2c17cef1 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -12,27 +12,24 @@ def initialize(auto_retry) @auto_retry_attempt_counter = 0 end - def download(target_locale, glotpress_url, is_source) + def download(glotpress_url) uri = URI(glotpress_url) response = Net::HTTP.get_response(uri) case response.code - when '200' - # All good pass the result along + when '200' # All good pass the result along response - when '301' - # Follow the redirect - UI.message("Received 301 for `#{locale}`. Following redirect...") - download(locale, response.header['location'], is_source) - when '429' - # We got rate-limited, auto_retry or offer to try again with a prompt + when '301' # Follow the redirect + UI.message("Received 301 for `#{response.uri}`. Following redirect...") + download(response.header['location']) + when '429' # We got rate-limited, auto_retry or offer to try again with a prompt if @auto_retry && @auto_retry_attempt_counter <= MAX_AUTO_RETRY_ATTEMPTS UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{AUTO_RETRY_SLEEP_TIME} seconds...") sleep(AUTO_RETRY_SLEEP_TIME) @auto_retry_attempt_counter += 1 - download(target_locale, response.uri, is_source) + download(response.uri) elsif UI.confirm("Retry downloading `#{response.uri}` after receiving 429 from the API?") - download(target_locale, response.uri, is_source) + download(response.uri) else UI.error("Abandoning `#{response.uri}` download as requested.") end @@ -64,7 +61,7 @@ def initialize(target_folder, target_files, auto_retry) # Downloads data from GlotPress, in JSON format def download(target_locale, glotpress_url, is_source) downloader = GlotPressDownloader.new(@auto_retry) - response = downloader.download(target_locale, glotpress_url, is_source) + response = downloader.download(glotpress_url) handle_glotpress_download(response: response, locale: target_locale, is_source: is_source) end From aa6bf79f80f11cb4c26d66662081c5033e23f760 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 17:11:38 +1100 Subject: [PATCH 08/22] Add test for `GlotPressDownaloader` --- spec/metadata_download_helper_spec.rb | 33 +++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/spec/metadata_download_helper_spec.rb b/spec/metadata_download_helper_spec.rb index ee7e71989..887ebb2eb 100644 --- a/spec/metadata_download_helper_spec.rb +++ b/spec/metadata_download_helper_spec.rb @@ -1,11 +1,40 @@ require 'spec_helper' +describe Fastlane::Helper::GlotPressDownloader do + describe 'downloading' do + context 'when GlotPress returs a 429 code' do + it 'automatically retries' do + downloader = described_class.new(true) + fake_url = 'https://test.com' + + count = 0 + stub_request(:get, fake_url).to_return do + count += 1 + if count == 1 + { status: 429, body: 'Too Many Requests' } + else + { status: 200, body: 'OK' } + end + end + + expect(Fastlane::UI).to receive(:message) + .with(/Received 429 for `#{fake_url}`. Auto retrying in 20 seconds.../) + + response = downloader.download(fake_url) + + expect(count).to eq(2) + expect(response.code).to eq('200') + end + end + end +end + describe Fastlane::Helper::MetadataDownloader do describe 'downloading from GlotPress' do context 'when GlotPress returs a 429 code' do it 'automatically retries' do in_tmp_dir do |tmp_dir| - metadata_downaloder = described_class.new( + metadata_downloader = described_class.new( tmp_dir, { key: { desc: 'target-file-name.txt' } }, true @@ -32,7 +61,7 @@ expect(Fastlane::UI).to receive(:success) .with(/Successfully downloaded `en-AU`./) - metadata_downaloder.download('en-AU', fake_url, false) + metadata_downloader.download('en-AU', fake_url, false) expect(count).to eq(2) end From 975dc46fb8147a683ef6e7c8236daba5a248bbea Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 17:13:11 +1100 Subject: [PATCH 09/22] Move `GlotpressDownloader` tests in dedicated file --- .../helper/metadata_download_helper.rb | 2 +- spec/metadata_download_helper_spec.rb | 29 ------------------- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index e2c17cef1..ebc3eb8ca 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -3,7 +3,7 @@ module Fastlane module Helper - class GlotPressDownloader + class GlotpressDownloader AUTO_RETRY_SLEEP_TIME = 20 MAX_AUTO_RETRY_ATTEMPTS = 30 diff --git a/spec/metadata_download_helper_spec.rb b/spec/metadata_download_helper_spec.rb index 887ebb2eb..7c85d8e19 100644 --- a/spec/metadata_download_helper_spec.rb +++ b/spec/metadata_download_helper_spec.rb @@ -1,34 +1,5 @@ require 'spec_helper' -describe Fastlane::Helper::GlotPressDownloader do - describe 'downloading' do - context 'when GlotPress returs a 429 code' do - it 'automatically retries' do - downloader = described_class.new(true) - fake_url = 'https://test.com' - - count = 0 - stub_request(:get, fake_url).to_return do - count += 1 - if count == 1 - { status: 429, body: 'Too Many Requests' } - else - { status: 200, body: 'OK' } - end - end - - expect(Fastlane::UI).to receive(:message) - .with(/Received 429 for `#{fake_url}`. Auto retrying in 20 seconds.../) - - response = downloader.download(fake_url) - - expect(count).to eq(2) - expect(response.code).to eq('200') - end - end - end -end - describe Fastlane::Helper::MetadataDownloader do describe 'downloading from GlotPress' do context 'when GlotPress returs a 429 code' do From ed269cfd886346a1b93b86607f9f9e6e67cfba9a Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 17:15:25 +1100 Subject: [PATCH 10/22] Use label for `auto_retry` `GlotpressDownloader` init parameter --- .../wpmreleasetoolkit/helper/metadata_download_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index ebc3eb8ca..1b1821bd9 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -7,7 +7,7 @@ class GlotpressDownloader AUTO_RETRY_SLEEP_TIME = 20 MAX_AUTO_RETRY_ATTEMPTS = 30 - def initialize(auto_retry) + def initialize(auto_retry: true) @auto_retry = auto_retry @auto_retry_attempt_counter = 0 end @@ -60,7 +60,7 @@ def initialize(target_folder, target_files, auto_retry) # Downloads data from GlotPress, in JSON format def download(target_locale, glotpress_url, is_source) - downloader = GlotPressDownloader.new(@auto_retry) + downloader = GlotPressDownloader.new(auto_retry: @auto_retry) response = downloader.download(glotpress_url) handle_glotpress_download(response: response, locale: target_locale, is_source: is_source) end From 0c1b71b7bd80b96c9773973538d7db7c33fb35a2 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 17:19:11 +1100 Subject: [PATCH 11/22] Make sleep time and retries configurable in `GlotpressDownloader` --- .../helper/metadata_download_helper.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index 1b1821bd9..ad390f880 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -7,8 +7,14 @@ class GlotpressDownloader AUTO_RETRY_SLEEP_TIME = 20 MAX_AUTO_RETRY_ATTEMPTS = 30 - def initialize(auto_retry: true) + def initialize( + auto_retry: true, + auto_retry_sleep_time: 20, + auto_retry_max_attempts: 30 + ) @auto_retry = auto_retry + @auto_retry_sleep_time = auto_retry_sleep_time + @auto_retry_max_attempts = auto_retry_max_attempts @auto_retry_attempt_counter = 0 end @@ -24,8 +30,8 @@ def download(glotpress_url) download(response.header['location']) when '429' # We got rate-limited, auto_retry or offer to try again with a prompt if @auto_retry && @auto_retry_attempt_counter <= MAX_AUTO_RETRY_ATTEMPTS - UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{AUTO_RETRY_SLEEP_TIME} seconds...") - sleep(AUTO_RETRY_SLEEP_TIME) + UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{@auto_retry_sleep_time} seconds...") + sleep(@auto_retry_sleep_time) @auto_retry_attempt_counter += 1 download(response.uri) elsif UI.confirm("Retry downloading `#{response.uri}` after receiving 429 from the API?") From cd87e7b7f1717fd577f7be95f52dc3f1caa5534a Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 Oct 2023 17:35:18 +1100 Subject: [PATCH 12/22] Add test for max retries and refine the implementation --- .../helper/metadata_download_helper.rb | 33 +++-------- spec/glotpress_downloader_spec.rb | 59 +++++++++++++++++++ 2 files changed, 68 insertions(+), 24 deletions(-) create mode 100644 spec/glotpress_downloader_spec.rb diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index ad390f880..1d8016e68 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -29,11 +29,15 @@ def download(glotpress_url) UI.message("Received 301 for `#{response.uri}`. Following redirect...") download(response.header['location']) when '429' # We got rate-limited, auto_retry or offer to try again with a prompt - if @auto_retry && @auto_retry_attempt_counter <= MAX_AUTO_RETRY_ATTEMPTS - UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{@auto_retry_sleep_time} seconds...") - sleep(@auto_retry_sleep_time) - @auto_retry_attempt_counter += 1 - download(response.uri) + if @auto_retry + if @auto_retry_attempt_counter < @auto_retry_max_attempts + UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{@auto_retry_sleep_time} seconds...") + sleep(@auto_retry_sleep_time) + @auto_retry_attempt_counter += 1 + download(response.uri) + else + UI.error("Abandoning `#{response.uri}` download after #{@auto_retry_attempt_counter} retries.") + end elsif UI.confirm("Retry downloading `#{response.uri}` after receiving 429 from the API?") download(response.uri) else @@ -51,9 +55,6 @@ def download(glotpress_url) module Fastlane module Helper class MetadataDownloader - AUTO_RETRY_SLEEP_TIME = 20 - MAX_AUTO_RETRY_ATTEMPTS = 30 - attr_reader :target_folder, :target_files def initialize(target_folder, target_files, auto_retry) @@ -159,22 +160,6 @@ def handle_glotpress_download(response:, locale:, is_source:) loc_data = JSON.parse(response.body) rescue loc_data = nil parse_data(locale, loc_data, is_source) reparse_alternates(target_locale, loc_data, is_source) unless @alternates.empty? - when '301' - # Follow the redirect - UI.message("Received 301 for `#{locale}`. Following redirect...") - download(locale, response.header['location'], is_source) - when '429' - # We got rate-limited, auto_retry or offer to try again with a prompt - if @auto_retry && @auto_retry_attempt_counter <= MAX_AUTO_RETRY_ATTEMPTS - UI.message("Received 429 for `#{locale}`. Auto retrying in #{AUTO_RETRY_SLEEP_TIME} seconds...") - sleep(AUTO_RETRY_SLEEP_TIME) - @auto_retry_attempt_counter += 1 - download(locale, response.uri, is_source) - elsif UI.confirm("Retry downloading `#{locale}` after receiving 429 from the API?") - download(locale, response.uri, is_source) - else - UI.error("Abandoning `#{locale}` download as requested.") - end else message = "Received unexpected #{response.code} from request to URI #{response.uri}." UI.abort_with_message!(message) unless UI.confirm("#{message} Continue anyway?") diff --git a/spec/glotpress_downloader_spec.rb b/spec/glotpress_downloader_spec.rb new file mode 100644 index 000000000..56814712e --- /dev/null +++ b/spec/glotpress_downloader_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Fastlane::Helper::GlotpressDownloader do + describe 'downloading' do + context 'when auto retry is enabled' do + context 'when GlotPress returs a 429 code' do + it 'retries automatically' do + sleep_time = 0.1 + downloader = described_class.new(auto_retry: true, auto_retry_sleep_time: sleep_time) + fake_url = 'https://test.com' + + count = 0 + stub_request(:get, fake_url).to_return do + count += 1 + if count == 1 + { status: 429, body: 'Too Many Requests' } + else + { status: 200, body: 'OK' } + end + end + + expect(Fastlane::UI).to receive(:message) + .with(/Received 429 for `#{fake_url}`. Auto retrying in #{sleep_time} seconds.../) + + response = downloader.download(fake_url) + + expect(count).to eq(2) + expect(response.code).to eq('200') + end + + context 'when the maximum number of retries is reached' do + it 'aborts' do + sleep_time = 0.1 + max_retries = 3 + downloader = described_class.new(auto_retry: true, auto_retry_sleep_time: sleep_time, auto_retry_max_attempts: max_retries) + fake_url = 'https://test.com' + + count = 0 + stub_request(:get, fake_url).to_return do + count += 1 + { status: 429, body: 'Too Many Requests' } + end + + expect(Fastlane::UI).to receive(:message) + .with(/Received 429 for `#{fake_url}`. Auto retrying in #{sleep_time} seconds.../) + .exactly(max_retries).times + + expect(Fastlane::UI).to receive(:error) + .with(/Abandoning `#{fake_url}` download after #{max_retries} retries./) + + downloader.download(fake_url) + + expect(count).to eq(max_retries + 1) # the original request plus the retries + end + end + end + end + end +end From 843381002efe3811a24b3b594d2df3bcdc5ec6e0 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 5 Oct 2023 14:08:08 +1100 Subject: [PATCH 13/22] Add test for GlotPress 429 with manual retry --- spec/glotpress_downloader_spec.rb | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/spec/glotpress_downloader_spec.rb b/spec/glotpress_downloader_spec.rb index 56814712e..734f38b5a 100644 --- a/spec/glotpress_downloader_spec.rb +++ b/spec/glotpress_downloader_spec.rb @@ -55,5 +55,30 @@ end end end + + context 'when auto retry is disabled' do + context 'when GlotPress returs a 429 code' do + it 'prompt the user for confirmation, ignoring the max auto retry parameter' do + sleep_time = 0.1 + max_retries = 2 + downloader = described_class.new(auto_retry: false, auto_retry_sleep_time: sleep_time, auto_retry_max_attempts: max_retries) + fake_url = 'https://test.com' + + stub_request(:get, fake_url).to_return(status: 429, body: 'Too Many Requests') + + count = 0 + allow(Fastlane::UI).to receive(:confirm).with("Retry downloading `#{fake_url}` after receiving 429 from the API?") do + count += 1 + # Simulate user retrying more times that the max retries, then aborting + count <= max_retries + end + + # When the user aborts, with finish with an error + expect(Fastlane::UI).to receive(:error).with(/Abandoning `#{fake_url}` download as requested./) + + downloader.download(fake_url) + end + end + end end end From 7f3ccb554cf949c79bb1b169b5d85fb9d1dd5a64 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 5 Oct 2023 14:14:30 +1100 Subject: [PATCH 14/22] Add test for 200 behavior in GlotPress download --- spec/glotpress_downloader_spec.rb | 36 +++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/spec/glotpress_downloader_spec.rb b/spec/glotpress_downloader_spec.rb index 734f38b5a..a0b1a5c88 100644 --- a/spec/glotpress_downloader_spec.rb +++ b/spec/glotpress_downloader_spec.rb @@ -3,6 +3,22 @@ describe Fastlane::Helper::GlotpressDownloader do describe 'downloading' do context 'when auto retry is enabled' do + context 'when GlotPress returs a 200 code' do + it 'returns the response, without retrying' do + downloader = described_class.new(auto_retry: true) + fake_url = 'https://test.com' + + stub_request(:get, fake_url).to_return(status: 200, body: 'OK') + + response = downloader.download(fake_url) + + expect(response.code).to eq('200') + expect(response.body).to eq('OK') + # Make sure there was no retry + assert_requested(:get, fake_url, times: 1) + end + end + context 'when GlotPress returs a 429 code' do it 'retries automatically' do sleep_time = 0.1 @@ -24,7 +40,7 @@ response = downloader.download(fake_url) - expect(count).to eq(2) + assert_requested(:get, fake_url, times: 2) expect(response.code).to eq('200') end @@ -50,13 +66,29 @@ downloader.download(fake_url) - expect(count).to eq(max_retries + 1) # the original request plus the retries + assert_requested(:get, fake_url, times: max_retries + 1) # the original request plus the retries end end end end context 'when auto retry is disabled' do + context 'when GlotPress returs a 200 code' do + it 'returns the response, without retrying' do + downloader = described_class.new(auto_retry: false) + fake_url = 'https://test.com' + + stub_request(:get, fake_url).to_return(status: 200, body: 'OK') + + response = downloader.download(fake_url) + + expect(response.code).to eq('200') + expect(response.body).to eq('OK') + # Make sure there was no retry + assert_requested(:get, fake_url, times: 1) + end + end + context 'when GlotPress returs a 429 code' do it 'prompt the user for confirmation, ignoring the max auto retry parameter' do sleep_time = 0.1 From cd5ef5192ad4a1e6f07e9bdc1fbc73464b0a57c4 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 5 Oct 2023 14:21:19 +1100 Subject: [PATCH 15/22] Make sleep time configurable in `MetadataDownloader` This way, the tests can run fast, instead of waiting for the default 20s --- .../wpmreleasetoolkit/helper/metadata_download_helper.rb | 8 ++++++-- spec/metadata_download_helper_spec.rb | 7 ++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index 1d8016e68..adbcd46c5 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -57,17 +57,21 @@ module Helper class MetadataDownloader attr_reader :target_folder, :target_files - def initialize(target_folder, target_files, auto_retry) + def initialize(target_folder, target_files, auto_retry, auto_retry_sleep_time = 20, auto_retry_max_attempts = 30) @target_folder = target_folder @target_files = target_files @auto_retry = auto_retry + @auto_retry_sleep_time = auto_retry_sleep_time @alternates = {} @auto_retry_attempt_counter = 0 end # Downloads data from GlotPress, in JSON format def download(target_locale, glotpress_url, is_source) - downloader = GlotPressDownloader.new(auto_retry: @auto_retry) + downloader = GlotpressDownloader.new( + auto_retry: @auto_retry, + auto_retry_sleep_time: @auto_retry_sleep_time + ) response = downloader.download(glotpress_url) handle_glotpress_download(response: response, locale: target_locale, is_source: is_source) end diff --git a/spec/metadata_download_helper_spec.rb b/spec/metadata_download_helper_spec.rb index 7c85d8e19..9a7a53cb8 100644 --- a/spec/metadata_download_helper_spec.rb +++ b/spec/metadata_download_helper_spec.rb @@ -8,7 +8,8 @@ metadata_downloader = described_class.new( tmp_dir, { key: { desc: 'target-file-name.txt' } }, - true + true, + 0.1 ) fake_url = 'https://test.com' @@ -24,7 +25,7 @@ end expect(Fastlane::UI).to receive(:message) - .with(/Received 429 for `#{fake_url}`. Auto retrying in 20 seconds.../) + .with(/Received 429 for `#{fake_url}`. Auto retrying in 0.1 seconds.../) expect(Fastlane::UI).to receive(:message) .with(/No translation available for en-AU/) @@ -34,7 +35,7 @@ metadata_downloader.download('en-AU', fake_url, false) - expect(count).to eq(2) + assert_requested(:get, fake_url, times: 2) end end end From d35d027f2020b73e8b3028c482f90b72df5abc54 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 5 Oct 2023 14:25:25 +1100 Subject: [PATCH 16/22] Move `GlotpressDownaloder` in dedicated file --- .../helper/glotpress_downloader.rb | 52 +++++++++++++++++++ .../helper/ios/ios_l10n_helper.rb | 4 +- .../helper/metadata_download_helper.rb | 51 ------------------ 3 files changed, 54 insertions(+), 53 deletions(-) create mode 100644 lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb new file mode 100644 index 000000000..74f690e7d --- /dev/null +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb @@ -0,0 +1,52 @@ +require 'net/http' + +module Fastlane + module Helper + class GlotpressDownloader + AUTO_RETRY_SLEEP_TIME = 20 + MAX_AUTO_RETRY_ATTEMPTS = 30 + + def initialize( + auto_retry: true, + auto_retry_sleep_time: 20, + auto_retry_max_attempts: 30 + ) + @auto_retry = auto_retry + @auto_retry_sleep_time = auto_retry_sleep_time + @auto_retry_max_attempts = auto_retry_max_attempts + @auto_retry_attempt_counter = 0 + end + + def download(glotpress_url) + uri = URI(glotpress_url) + response = Net::HTTP.get_response(uri) + + case response.code + when '200' # All good pass the result along + response + when '301' # Follow the redirect + UI.message("Received 301 for `#{response.uri}`. Following redirect...") + download(response.header['location']) + when '429' # We got rate-limited, auto_retry or offer to try again with a prompt + if @auto_retry + if @auto_retry_attempt_counter < @auto_retry_max_attempts + UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{@auto_retry_sleep_time} seconds...") + sleep(@auto_retry_sleep_time) + @auto_retry_attempt_counter += 1 + download(response.uri) + else + UI.error("Abandoning `#{response.uri}` download after #{@auto_retry_attempt_counter} retries.") + end + elsif UI.confirm("Retry downloading `#{response.uri}` after receiving 429 from the API?") + download(response.uri) + else + UI.error("Abandoning `#{response.uri}` download as requested.") + end + else + message = "Received unexpected #{response.code} from request to URI #{response.uri}." + UI.abort_with_message!(message) unless UI.confirm("#{message} Continue anyway?") + end + end + end + end +end diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb index ab8175d3e..7f6c73558 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb @@ -168,8 +168,8 @@ def self.download_glotpress_export_file( destination:, autoretry: true, autoretry_count: 0, - autoretry_max: Fastlane::Helper::MetadataDownloader::MAX_AUTO_RETRY_ATTEMPTS, - autoretry_sleep: Fastlane::Helper::MetadataDownloader::AUTO_RETRY_SLEEP_TIME + autoretry_max: Fastlane::Helper::GlotpressDownloader::MAX_AUTO_RETRY_ATTEMPTS, + autoretry_sleep: Fastlane::Helper::GlotpressDownloader::AUTO_RETRY_SLEEP_TIME ) query_params = (filters || {}).transform_keys { |k| "filters[#{k}]" }.merge(format: 'strings') uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations/?#{URI.encode_www_form(query_params)}") diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index adbcd46c5..bd3294261 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -1,57 +1,6 @@ require 'net/http' require 'json' -module Fastlane - module Helper - class GlotpressDownloader - AUTO_RETRY_SLEEP_TIME = 20 - MAX_AUTO_RETRY_ATTEMPTS = 30 - - def initialize( - auto_retry: true, - auto_retry_sleep_time: 20, - auto_retry_max_attempts: 30 - ) - @auto_retry = auto_retry - @auto_retry_sleep_time = auto_retry_sleep_time - @auto_retry_max_attempts = auto_retry_max_attempts - @auto_retry_attempt_counter = 0 - end - - def download(glotpress_url) - uri = URI(glotpress_url) - response = Net::HTTP.get_response(uri) - - case response.code - when '200' # All good pass the result along - response - when '301' # Follow the redirect - UI.message("Received 301 for `#{response.uri}`. Following redirect...") - download(response.header['location']) - when '429' # We got rate-limited, auto_retry or offer to try again with a prompt - if @auto_retry - if @auto_retry_attempt_counter < @auto_retry_max_attempts - UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{@auto_retry_sleep_time} seconds...") - sleep(@auto_retry_sleep_time) - @auto_retry_attempt_counter += 1 - download(response.uri) - else - UI.error("Abandoning `#{response.uri}` download after #{@auto_retry_attempt_counter} retries.") - end - elsif UI.confirm("Retry downloading `#{response.uri}` after receiving 429 from the API?") - download(response.uri) - else - UI.error("Abandoning `#{response.uri}` download as requested.") - end - else - message = "Received unexpected #{response.code} from request to URI #{response.uri}." - UI.abort_with_message!(message) unless UI.confirm("#{message} Continue anyway?") - end - end - end - end -end - module Fastlane module Helper class MetadataDownloader From 80ddf812f4453fa20ca84aa4575a231a43049376 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 5 Oct 2023 20:52:08 +1100 Subject: [PATCH 17/22] Use new `GlotpressDownloader` in `ios_l10n_helper` --- .../helper/glotpress_downloader.rb | 4 +-- .../helper/ios/ios_l10n_helper.rb | 34 ++++++------------- spec/ios_l10n_helper_spec.rb | 6 ++-- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb index 74f690e7d..3f674f62d 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb @@ -17,9 +17,9 @@ def initialize( @auto_retry_attempt_counter = 0 end - def download(glotpress_url) + def download(glotpress_url, options = {}) uri = URI(glotpress_url) - response = Net::HTTP.get_response(uri) + response = Net::HTTP.get_response(uri, options) case response.code when '200' # All good pass the result along diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb index 7f6c73558..55c59f8e2 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb @@ -174,35 +174,21 @@ def self.download_glotpress_export_file( query_params = (filters || {}).transform_keys { |k| "filters[#{k}]" }.merge(format: 'strings') uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations/?#{URI.encode_www_form(query_params)}") + downloader = Fastlane::Helper::GlotpressDownloader.new( + auto_retry: autoretry, + auto_retry_sleep_time: autoretry_sleep, + auto_retry_max_attempts: autoretry_max + ) + # Set an unambiguous User Agent so GlotPress won't rate-limit us options = { 'User-Agent' => Wpmreleasetoolkit::USER_AGENT } + response = downloader.download(uri.to_s, options) + begin - IO.copy_stream(uri.open(options), destination) + IO.copy_stream(StringIO.new(response.body), destination) rescue StandardError => e - UI.error "Error downloading locale `#{locale}` — #{e.message} (#{uri})" - if e.is_a?(OpenURI::HTTPError) - if e.io.status[0] == '429' && autoretry - if autoretry_count < autoretry_max - UI.message("Received 429 for `#{locale}`. Auto retrying in #{autoretry_sleep} seconds...") - sleep(autoretry_sleep) - return download_glotpress_export_file( - project_url: project_url, - locale: locale, - filters: filters, - destination: destination, - autoretry: autoretry, - autoretry_count: autoretry_count + 1, - autoretry_max: autoretry_max - ) - else - UI.user_error!("Abandoning `#{locale}` download after #{autoretry_max} attempts.") - end - elsif UI.confirm("Retry downloading `#{locale}`?") - retry - end - end - return nil + UI.error "Error saving locale `#{locale}` — #{e.message} (#{uri})" end end end diff --git a/spec/ios_l10n_helper_spec.rb b/spec/ios_l10n_helper_spec.rb index 3d8333535..38a8657d4 100644 --- a/spec/ios_l10n_helper_spec.rb +++ b/spec/ios_l10n_helper_spec.rb @@ -259,14 +259,14 @@ def file_encoding(path) # Arrange stub = stub_request(:get, "#{gp_fake_url}/invalid/default/export-translations/").with(query: { format: 'strings' }).to_return(status: [404, 'Not Found']) error_messages = [] - allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) } + allow(FastlaneCore::UI).to receive(:abort_with_message!) { |message| error_messages.append(message) } allow(FastlaneCore::UI).to receive(:confirm).and_return(false) # as we will be asked if we want to retry when getting a network error dest = StringIO.new # Act described_class.download_glotpress_export_file(project_url: gp_fake_url, locale: 'invalid', filters: nil, destination: dest) # Assert expect(stub).to have_been_made.once - expect(error_messages).to eq(["Error downloading locale `invalid` — 404 Not Found (#{gp_fake_url}/invalid/default/export-translations/?format=strings)"]) + expect(error_messages).to eq(["Error downloading locale `invalid` — Received unexpected 404 from request to URI #{gp_fake_url}/invalid/default/export-translations/?format=strings"]) end it 'prints an `UI.error` if the destination cannot be written to' do @@ -280,7 +280,7 @@ def file_encoding(path) # Assert expect(stub).to have_been_made.once expect(File).not_to exist(dest) - expect(error_messages).to eq(["Error downloading locale `fr` — No such file or directory @ rb_sysopen - #{dest} (#{gp_fake_url}/fr/default/export-translations/?format=strings)"]) + expect(error_messages).to eq(["Error saving locale `fr` — No such file or directory @ rb_sysopen - #{dest} (#{gp_fake_url}/fr/default/export-translations/?format=strings)"]) end end end From 71077a8378a12ea3c8946410e9332b81b1d91d82 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 6 Oct 2023 14:00:18 +1100 Subject: [PATCH 18/22] Add missing `options` in recursive `GlotpressDownloader` call --- .../plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb index 3f674f62d..17e00e3f9 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb @@ -33,12 +33,12 @@ def download(glotpress_url, options = {}) UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{@auto_retry_sleep_time} seconds...") sleep(@auto_retry_sleep_time) @auto_retry_attempt_counter += 1 - download(response.uri) + download(response.uri, options) else UI.error("Abandoning `#{response.uri}` download after #{@auto_retry_attempt_counter} retries.") end elsif UI.confirm("Retry downloading `#{response.uri}` after receiving 429 from the API?") - download(response.uri) + download(response.uri, options) else UI.error("Abandoning `#{response.uri}` download as requested.") end From 48896c863300559763cfdc89644d16ae46e4b656 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 6 Oct 2023 14:00:36 +1100 Subject: [PATCH 19/22] Remove an unused `autoretry_count` parameter --- .../plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb index 55c59f8e2..97d08efa3 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb @@ -167,7 +167,6 @@ def self.download_glotpress_export_file( filters:, destination:, autoretry: true, - autoretry_count: 0, autoretry_max: Fastlane::Helper::GlotpressDownloader::MAX_AUTO_RETRY_ATTEMPTS, autoretry_sleep: Fastlane::Helper::GlotpressDownloader::AUTO_RETRY_SLEEP_TIME ) From 0330b3b61652f5666fed09de063be581e2daff21 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 6 Oct 2023 14:00:58 +1100 Subject: [PATCH 20/22] Remove an unused `auto_retry_max_attempts` parameter --- .../plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb index bd3294261..231224e42 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb @@ -6,7 +6,7 @@ module Helper class MetadataDownloader attr_reader :target_folder, :target_files - def initialize(target_folder, target_files, auto_retry, auto_retry_sleep_time = 20, auto_retry_max_attempts = 30) + def initialize(target_folder, target_files, auto_retry, auto_retry_sleep_time = 20) @target_folder = target_folder @target_files = target_files @auto_retry = auto_retry From 0847fc9b52c511630372693d97d9b2a4aa22eb42 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 6 Oct 2023 14:20:26 +1100 Subject: [PATCH 21/22] Add test for metadata downloader file saving behavior While I was at it... Also useful to ensure the new downloader object didn't break the existing download logic. --- spec/metadata_download_helper_spec.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/spec/metadata_download_helper_spec.rb b/spec/metadata_download_helper_spec.rb index 9a7a53cb8..45cb1feb2 100644 --- a/spec/metadata_download_helper_spec.rb +++ b/spec/metadata_download_helper_spec.rb @@ -2,6 +2,27 @@ describe Fastlane::Helper::MetadataDownloader do describe 'downloading from GlotPress' do + it 'save the downloaded data to the target folder and file, including locale name' do + in_tmp_dir do |tmp_dir| + destination_name = 'target-file-name.txt' + dummy_url = 'https://test.com' + metadata_downloader = described_class.new(tmp_dir, { key: { desc: destination_name } }, true) + + stub_request(:get, dummy_url).to_return( + status: 200, + # GlotPress responses have a custom format. + body: { "key\u0004test metadata" => ['test metadata'] }.to_json + ) + + metadata_downloader.download('en-AU', dummy_url, false) + + destination_path_with_locale = File.join(tmp_dir, 'en-AU', destination_name) + expect(File.exist?(destination_path_with_locale)).to be true + # We also expect a trailing new line. + expect(File.read(destination_path_with_locale)).to eq("test metadata\n") + end + end + context 'when GlotPress returs a 429 code' do it 'automatically retries' do in_tmp_dir do |tmp_dir| From 65801ac2d2b8c06ade3f3ab60f8693446ee23c76 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 6 Oct 2023 14:27:54 +1100 Subject: [PATCH 22/22] Add changelog entry for new 429 auto retry behavior --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77c08e3d6..2c5f13307 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ _None_ ### New Features -_None_ +- `ios_download_string_files_from_glotpress` supports automatic retry when receiving a 429 response, enabled by default [#516] ### Bug Fixes