Merge branch 'github-rate-limit-on-project-import' into 'master'

Project import Github API rate limit is exceeded

See merge request gitlab-org/gitlab!28785

app/controllers/import/github_controller.rb

@@ -9,6 +9,7 @@ class Import::GithubController < Import::BaseController
   before_action :expire_etag_cache, only: [:status, :create]
 
   rescue_from Octokit::Unauthorized, with: :provider_unauthorized
+  rescue_from Octokit::TooManyRequests, with: :provider_rate_limit
 
   def new
     if !ci_cd_only? && github_import_configured? && logged_in_with_provider?
@@ -142,6 +143,13 @@ class Import::GithubController < Import::BaseController
       alert: "Access denied to your #{Gitlab::ImportSources.title(provider.to_s)} account."
   end
 
+  def provider_rate_limit(exception)
+    reset_time = Time.at(exception.response_headers['x-ratelimit-reset'].to_i)
+    session[access_token_key] = nil
+    redirect_to new_import_url,
+      alert: _("GitHub API rate limit exceeded. Try again after %{reset_time}") % { reset_time: reset_time }
+  end
+
   def access_token_key
     :"#{provider}_access_token"
   end
@@ -180,7 +188,7 @@ class Import::GithubController < Import::BaseController
   end
 
   def client_options
-    {}
+    { wait_for_rate_limit_reset: false }
   end
 
   def extra_import_params

changelogs/unreleased/github-rate-limit-on-project-import.yml

+---
+title: Better error message when importing a Github project and Github API rate limit is exceeded
+merge_request: 28785
+author:
+type: fixed

lib/gitlab/legacy_github_import/client.rb

@@ -6,13 +6,14 @@ module Gitlab
       GITHUB_SAFE_REMAINING_REQUESTS = 100
       GITHUB_SAFE_SLEEP_TIME = 500
 
-      attr_reader :access_token, :host, :api_version
+      attr_reader :access_token, :host, :api_version, :wait_for_rate_limit_reset
 
-      def initialize(access_token, host: nil, api_version: 'v3')
+      def initialize(access_token, host: nil, api_version: 'v3', wait_for_rate_limit_reset: true)
         @access_token = access_token
         @host = host.to_s.sub(%r{/+\z}, '')
         @api_version = api_version
         @users = {}
+        @wait_for_rate_limit_reset = wait_for_rate_limit_reset
 
         if access_token
           ::Octokit.auto_paginate = false
@@ -120,7 +121,7 @@ module Gitlab
       end
 
       def request(method, *args, &block)
-        sleep rate_limit_sleep_time if rate_limit_exceed?
+        sleep rate_limit_sleep_time if wait_for_rate_limit_reset && rate_limit_exceed?
 
         data = api.__send__(method, *args) # rubocop:disable GitlabSecurity/PublicSend
         return data unless data.is_a?(Array)

locale/gitlab.pot

@@ -9726,6 +9726,9 @@ msgstr ""
 msgid "Git version"
 msgstr ""
 
+msgid "GitHub API rate limit exceeded. Try again after %{reset_time}"
+msgstr ""
+
 msgid "GitHub import"
 msgstr ""
 

spec/lib/gitlab/legacy_github_import/client_spec.rb

@@ -5,8 +5,9 @@ require 'spec_helper'
 describe Gitlab::LegacyGithubImport::Client do
   let(:token) { '123456' }
   let(:github_provider) { Settingslogic.new('app_id' => 'asd123', 'app_secret' => 'asd123', 'name' => 'github', 'args' => { 'client_options' => {} }) }
+  let(:wait_for_rate_limit_reset) { true }
 
-  subject(:client) { described_class.new(token) }
+  subject(:client) { described_class.new(token, wait_for_rate_limit_reset: wait_for_rate_limit_reset) }
 
   before do
     allow(Gitlab.config.omniauth).to receive(:providers).and_return([github_provider])
@@ -88,10 +89,23 @@ describe Gitlab::LegacyGithubImport::Client do
     end
   end
 
+  context 'github rate limit' do
     it 'does not raise error when rate limit is disabled' do
       stub_request(:get, /api.github.com/)
       allow(client.api).to receive(:rate_limit!).and_raise(Octokit::NotFound)
 
-    expect { client.issues {} }.not_to raise_error
+      expect { client.repos }.not_to raise_error
+    end
+
+    context 'when wait for rate limit is disabled' do
+      let(:wait_for_rate_limit_reset) { false }
+
+      it 'raises the error limit error when requested' do
+        stub_request(:get, /api.github.com/)
+        allow(client.api).to receive(:repos).and_raise(Octokit::TooManyRequests)
+
+        expect { client.repos }.to raise_error(Octokit::TooManyRequests)
+      end
+    end
   end
 end