Merge branch 'gh-rate-limit' into 'master'

Wrap all rate limiting logic inside GitHub API client

## What does this MR do?

Move the actual rate limiting logic to GitHub API to clean the code inside the GitHub importer, and avoid code duplication.

## Are there points in the code the reviewer needs to double check?

No there aren't.

## Why was this MR needed?

Avoid code duplication to handle API rate limit in every call to the GitHub API.

## What are the relevant issue numbers?

There are none.

## Screenshots (if relevant)

Not relevant.

See merge request !4552

2 files changed, 40 insertions, 57 deletions

diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb
index 67988ea3460..d325eca6d99 100644
--- a/lib/gitlab/github_import/client.rb
+++ b/lib/gitlab/github_import/client.rb
@@ -1,6 +1,9 @@
 module Gitlab
   module GithubImport
     class Client
+      GITHUB_SAFE_REMAINING_REQUESTS = 100
+      GITHUB_SAFE_SLEEP_TIME = 500
+
       attr_reader :client, :api
 
       def initialize(access_token)
@@ -11,7 +14,7 @@ module Gitlab
         )
 
         if access_token
-          ::Octokit.auto_paginate = true
+          ::Octokit.auto_paginate = false
 
           @api = ::Octokit::Client.new(
             access_token: access_token,
@@ -36,7 +39,7 @@ module Gitlab
 
       def method_missing(method, *args, &block)
         if api.respond_to?(method)
-          api.send(method, *args, &block)
+          request { api.send(method, *args, &block) }
         else
           super(method, *args, &block)
         end
@@ -55,6 +58,34 @@ module Gitlab
       def github_options
         config["args"]["client_options"].deep_symbolize_keys
       end
+
+      def rate_limit
+        api.rate_limit!
+      end
+
+      def rate_limit_exceed?
+        rate_limit.remaining <= GITHUB_SAFE_REMAINING_REQUESTS
+      end
+
+      def rate_limit_sleep_time
+        rate_limit.resets_in + GITHUB_SAFE_SLEEP_TIME
+      end
+
+      def request
+        sleep rate_limit_sleep_time if rate_limit_exceed?
+
+        data = yield
+
+        last_response = api.last_response
+
+        while last_response.rels[:next]
+          sleep rate_limit_sleep_time if rate_limit_exceed?
+          last_response = last_response.rels[:next].get
+          data.concat(last_response.data) if last_response.data.is_a?(Array)
+        end
+
+        data
+      end
     end
   end
 end
diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb
index 5ef9d66ba68..e5cf66a0371 100644
--- a/lib/gitlab/github_import/importer.rb
+++ b/lib/gitlab/github_import/importer.rb
@@ -3,9 +3,6 @@ module Gitlab
     class Importer
       include Gitlab::ShellAdapter
 
-      GITHUB_SAFE_REMAINING_REQUESTS = 100
-      GITHUB_SAFE_SLEEP_TIME = 500
-
       attr_reader :client, :project, :repo, :repo_url
 
       def initialize(project)
@@ -28,52 +25,12 @@ module Gitlab
 
       private
 
-      def turn_auto_pagination_off!
-        client.auto_paginate = false
-      end
-
-      def turn_auto_pagination_on!
-        client.auto_paginate = true
-      end
-
-      def rate_limit
-        client.rate_limit!
-      end
-
-      def rate_limit_exceed?
-        rate_limit.remaining <= GITHUB_SAFE_REMAINING_REQUESTS
-      end
-
-      def rate_limit_sleep_time
-        rate_limit.resets_in + GITHUB_SAFE_SLEEP_TIME
-      end
-
-      def paginate
-        turn_auto_pagination_off!
-
-        sleep rate_limit_sleep_time if rate_limit_exceed?
-
-        data = yield
-
-        last_response = client.last_response
-
-        while last_response.rels[:next]
-          sleep rate_limit_sleep_time if rate_limit_exceed?
-          last_response = last_response.rels[:next].get
-          data.concat(last_response.data) if last_response.data.is_a?(Array)
-        end
-
-        turn_auto_pagination_on!
-
-        data
-      end
-
       def credentials
         @credentials ||= project.import_data.credentials if project.import_data
       end
 
       def import_labels
-        labels = paginate { client.labels(repo, per_page: 100) }
+        labels = client.labels(repo, per_page: 100)
         labels.each { |raw| LabelFormatter.new(project, raw).create! }
 
         true
@@ -82,7 +39,7 @@ module Gitlab
       end
 
       def import_milestones
-        milestones = paginate { client.milestones(repo, state: :all, per_page: 100) }
+        milestones = client.milestones(repo, state: :all, per_page: 100)
         milestones.each { |raw| MilestoneFormatter.new(project, raw).create! }
 
         true
@@ -91,9 +48,9 @@ module Gitlab
       end
 
       def import_issues
-        data = paginate { client.issues(repo, state: :all, sort: :created, direction: :asc, per_page: 100) }
+        issues = client.issues(repo, state: :all, sort: :created, direction: :asc, per_page: 100)
 
-        data.each do |raw|
+        issues.each do |raw|
           gh_issue = IssueFormatter.new(project, raw)
 
           if gh_issue.valid?
@@ -112,7 +69,7 @@ module Gitlab
         hooks = client.hooks(repo).map { |raw| HookFormatter.new(raw) }.select(&:valid?)
         disable_webhooks(hooks)
 
-        pull_requests = paginate { client.pull_requests(repo, state: :all, sort: :created, direction: :asc, per_page: 100) }
+        pull_requests = client.pull_requests(repo, state: :all, sort: :created, direction: :asc, per_page: 100)
         pull_requests = pull_requests.map { |raw| PullRequestFormatter.new(project, raw) }.select(&:valid?)
 
         source_branches_removed = pull_requests.reject(&:source_branch_exists?).map { |pr| [pr.source_branch_name, pr.source_branch_sha] }
@@ -146,14 +103,12 @@ module Gitlab
 
       def update_webhooks(hooks, options)
         hooks.each do |hook|
-          sleep rate_limit_sleep_time if rate_limit_exceed?
           client.edit_hook(repo, hook.id, hook.name, hook.config, options)
         end
       end
 
       def restore_branches(branches)
         branches.each do |name, sha|
-          sleep rate_limit_sleep_time if rate_limit_exceed?
           client.create_ref(repo, "refs/heads/#{name}", sha)
         end
 
@@ -162,15 +117,12 @@ module Gitlab
 
       def clean_up_restored_branches(branches)
         branches.each do |name, _|
-          sleep rate_limit_sleep_time if rate_limit_exceed?
           client.delete_ref(repo, "heads/#{name}")
           project.repository.rm_branch(project.creator, name)
         end
       end
 
       def apply_labels(issuable)
-        sleep rate_limit_sleep_time if rate_limit_exceed?
-
         issue = client.issue(repo, issuable.iid)
 
         if issue.labels.count > 0
@@ -183,12 +135,12 @@ module Gitlab
       end
 
       def import_comments(issuable)
-        comments = paginate { client.issue_comments(repo, issuable.iid, per_page: 100) }
+        comments = client.issue_comments(repo, issuable.iid, per_page: 100)
         create_comments(issuable, comments)
       end
 
       def import_comments_on_diff(merge_request)
-        comments = paginate { client.pull_request_comments(repo, merge_request.iid, per_page: 100) }
+        comments = client.pull_request_comments(repo, merge_request.iid, per_page: 100)
         create_comments(merge_request, comments)
       end