Retrieve PR comments only when we know there are any

The Pull Request resource doesn't include the `comments` field, while
the Issue resource does. And since we're looping through all issues
anyway, we can freely check if the issue is a PR and has comments and in
this case only fetch comments for it.

That means if you have 1000 PRs but only 200 with comments, you will do
200 API requests instead of 1000. :notbad:

Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
Rémy Coutable 2017-09-21 15:30:12 +02:00
parent 60053c1201
commit e484a06472
2 changed files with 24 additions and 20 deletions

View file

@ -1,6 +1,7 @@
module Github module Github
class Client class Client
TIMEOUT = 60 TIMEOUT = 60
DEFAULT_PER_PAGE = 100
attr_reader :connection, :rate_limit attr_reader :connection, :rate_limit
@ -20,7 +21,7 @@ module Github
exceed, reset_in = rate_limit.get exceed, reset_in = rate_limit.get
sleep reset_in if exceed sleep reset_in if exceed
Github::Response.new(connection.get(url, query)) Github::Response.new(connection.get(url, { per_page: DEFAULT_PER_PAGE }.merge(query)))
end end
private private

View file

@ -115,7 +115,7 @@ module Github
url = "/repos/#{repo}/labels" url = "/repos/#{repo}/labels"
while url while url
response = Github::Client.new(options).get(url, per_page: 100) response = Github::Client.new(options).get(url)
response.body.each do |raw| response.body.each do |raw|
begin begin
@ -139,7 +139,7 @@ module Github
url = "/repos/#{repo}/milestones" url = "/repos/#{repo}/milestones"
while url while url
response = Github::Client.new(options).get(url, state: :all, per_page: 100) response = Github::Client.new(options).get(url, state: :all)
response.body.each do |raw| response.body.each do |raw|
begin begin
@ -168,7 +168,7 @@ module Github
url = "/repos/#{repo}/pulls" url = "/repos/#{repo}/pulls"
while url while url
response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc, per_page: 100) response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc)
response.body.each do |raw| response.body.each do |raw|
pull_request = Github::Representation::PullRequest.new(raw, options.merge(project: project)) pull_request = Github::Representation::PullRequest.new(raw, options.merge(project: project))
@ -202,13 +202,8 @@ module Github
merge_request.save!(validate: false) merge_request.save!(validate: false)
merge_request.merge_request_diffs.create merge_request.merge_request_diffs.create
# Fetch review comments
review_comments_url = "/repos/#{repo}/pulls/#{pull_request.iid}/comments" review_comments_url = "/repos/#{repo}/pulls/#{pull_request.iid}/comments"
fetch_comments(merge_request, :review_comment, review_comments_url, LegacyDiffNote) fetch_comments(merge_request, :review_comment, review_comments_url, LegacyDiffNote)
# Fetch comments
comments_url = "/repos/#{repo}/issues/#{pull_request.iid}/comments"
fetch_comments(merge_request, :comment, comments_url)
rescue => e rescue => e
error(:pull_request, pull_request.url, e.message) error(:pull_request, pull_request.url, e.message)
ensure ensure
@ -224,7 +219,7 @@ module Github
url = "/repos/#{repo}/issues" url = "/repos/#{repo}/issues"
while url while url
response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc, per_page: 100) response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc)
response.body.each { |raw| populate_issue(raw) } response.body.each { |raw| populate_issue(raw) }
@ -241,12 +236,17 @@ module Github
# for both features, like manipulating assignees, labels # for both features, like manipulating assignees, labels
# and milestones, are provided within the Issues API. # and milestones, are provided within the Issues API.
if representation.pull_request? if representation.pull_request?
return unless representation.has_labels? return if !representation.has_labels? && !representation.has_comments?
merge_request = MergeRequest.find_by!(target_project_id: project.id, iid: representation.iid) merge_request = MergeRequest.find_by!(target_project_id: project.id, iid: representation.iid)
merge_request.update_attribute(:label_ids, label_ids(representation.labels))
if representation.has_labels?
merge_request.update_attribute(:label_ids, label_ids(representation.labels))
end
fetch_comments_conditionally(merge_request, representation)
else else
return if Issue.where(iid: representation.iid, project_id: project.id).exists? return if Issue.exists?(iid: representation.iid, project_id: project.id)
author_id = user_id(representation.author, project.creator_id) author_id = user_id(representation.author, project.creator_id)
issue = Issue.new issue = Issue.new
@ -263,20 +263,23 @@ module Github
issue.updated_at = representation.updated_at issue.updated_at = representation.updated_at
issue.save!(validate: false) issue.save!(validate: false)
# Fetch comments fetch_comments_conditionally(issue, representation)
if representation.has_comments?
comments_url = "/repos/#{repo}/issues/#{issue.iid}/comments"
fetch_comments(issue, :comment, comments_url)
end
end end
rescue => e rescue => e
error(:issue, representation.url, e.message) error(:issue, representation.url, e.message)
end end
end end
def fetch_comments_conditionally(issuable, representation)
if representation.has_comments?
comments_url = "/repos/#{repo}/issues/#{issuable.iid}/comments"
fetch_comments(issuable, :comment, comments_url)
end
end
def fetch_comments(noteable, type, url, klass = Note) def fetch_comments(noteable, type, url, klass = Note)
while url while url
comments = Github::Client.new(options).get(url, per_page: 100) comments = Github::Client.new(options).get(url)
ActiveRecord::Base.no_touching do ActiveRecord::Base.no_touching do
comments.body.each do |raw| comments.body.each do |raw|
@ -308,7 +311,7 @@ module Github
url = "/repos/#{repo}/releases" url = "/repos/#{repo}/releases"
while url while url
response = Github::Client.new(options).get(url, per_page: 100) response = Github::Client.new(options).get(url)
response.body.each do |raw| response.body.each do |raw|
representation = Github::Representation::Release.new(raw) representation = Github::Representation::Release.new(raw)