Re-use the github PR object when fetching reviews
We inadvertently fetch the PR object twice because of the way we were fetching reviews. Keep it around so we can make one less API call. Co-Authored-By: Clark Boylan <cboylan@sapwetik.org> Change-Id: If5260278adb525566d99eedaecaf8b4f5077d43e
This commit is contained in:
parent
275cbc92e0
commit
6b3333527c
|
@ -784,6 +784,15 @@ class GithubChangeReference(git.Reference):
|
||||||
_points_to_commits_only = True
|
_points_to_commits_only = True
|
||||||
|
|
||||||
|
|
||||||
|
class FakeGHReview(object):
|
||||||
|
|
||||||
|
def __init__(self, data):
|
||||||
|
self.data = data
|
||||||
|
|
||||||
|
def as_dict(self):
|
||||||
|
return self.data
|
||||||
|
|
||||||
|
|
||||||
class FakeGithubPullRequest(object):
|
class FakeGithubPullRequest(object):
|
||||||
|
|
||||||
def __init__(self, github, number, project, branch,
|
def __init__(self, github, number, project, branch,
|
||||||
|
@ -1050,14 +1059,14 @@ class FakeGithubPullRequest(object):
|
||||||
submitted_at = time.strftime(
|
submitted_at = time.strftime(
|
||||||
gh_time_format, granted_on.timetuple())
|
gh_time_format, granted_on.timetuple())
|
||||||
|
|
||||||
self.reviews.append({
|
self.reviews.append(FakeGHReview({
|
||||||
'state': state,
|
'state': state,
|
||||||
'user': {
|
'user': {
|
||||||
'login': user,
|
'login': user,
|
||||||
'email': user + "@derp.com",
|
'email': user + "@derp.com",
|
||||||
},
|
},
|
||||||
'submitted_at': submitted_at,
|
'submitted_at': submitted_at,
|
||||||
})
|
}))
|
||||||
|
|
||||||
def getPRReference(self):
|
def getPRReference(self):
|
||||||
return '%s/head' % self.number
|
return '%s/head' % self.number
|
||||||
|
@ -1229,10 +1238,6 @@ class FakeGithubConnection(githubconnection.GithubConnection):
|
||||||
super(FakeGithubConnection, self).addProject(project)
|
super(FakeGithubConnection, self).addProject(project)
|
||||||
self.getGithubClient(project).addProject(project)
|
self.getGithubClient(project).addProject(project)
|
||||||
|
|
||||||
def _getPullReviews(self, owner, project, number):
|
|
||||||
pr = self.pull_requests[int(number)]
|
|
||||||
return pr.reviews
|
|
||||||
|
|
||||||
def getGitUrl(self, project):
|
def getGitUrl(self, project):
|
||||||
if self.git_url_with_auth:
|
if self.git_url_with_auth:
|
||||||
auth_token = ''.join(
|
auth_token = ''.join(
|
||||||
|
|
|
@ -296,6 +296,9 @@ class FakePull(object):
|
||||||
return [FakeFile(fn)
|
return [FakeFile(fn)
|
||||||
for fn in sorted(self._fake_pull_request.files)][:300]
|
for fn in sorted(self._fake_pull_request.files)][:300]
|
||||||
|
|
||||||
|
def reviews(self):
|
||||||
|
return self._fake_pull_request.reviews
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def head(self):
|
def head(self):
|
||||||
client = FakeGithubClient(self._fake_pull_request.github.github_data)
|
client = FakeGithubClient(self._fake_pull_request.github.github_data)
|
||||||
|
|
|
@ -417,7 +417,8 @@ class GithubEventProcessor(object):
|
||||||
def _issue_to_pull_request(self, body):
|
def _issue_to_pull_request(self, body):
|
||||||
number = body.get('issue').get('number')
|
number = body.get('issue').get('number')
|
||||||
project_name = body.get('repository').get('full_name')
|
project_name = body.get('repository').get('full_name')
|
||||||
pr_body = self.connection.getPull(project_name, number, self.log)
|
pr_body, pr_obj = self.connection.getPull(
|
||||||
|
project_name, number, self.log)
|
||||||
if pr_body is None:
|
if pr_body is None:
|
||||||
self.log.debug('Pull request #%s not found in project %s' %
|
self.log.debug('Pull request #%s not found in project %s' %
|
||||||
(number, project_name))
|
(number, project_name))
|
||||||
|
@ -963,7 +964,7 @@ class GithubConnection(BaseConnection):
|
||||||
|
|
||||||
def _updateChange(self, change):
|
def _updateChange(self, change):
|
||||||
self.log.info("Updating %s" % (change,))
|
self.log.info("Updating %s" % (change,))
|
||||||
change.pr = self.getPull(change.project.name, change.number)
|
change.pr, pr_obj = self.getPull(change.project.name, change.number)
|
||||||
change.ref = "refs/pull/%s/head" % change.number
|
change.ref = "refs/pull/%s/head" % change.number
|
||||||
change.branch = change.pr.get('base').get('ref')
|
change.branch = change.pr.get('base').get('ref')
|
||||||
|
|
||||||
|
@ -989,7 +990,7 @@ class GithubConnection(BaseConnection):
|
||||||
change.is_merged = change.pr.get('merged')
|
change.is_merged = change.pr.get('merged')
|
||||||
change.status = self._get_statuses(change.project,
|
change.status = self._get_statuses(change.project,
|
||||||
change.patchset)
|
change.patchset)
|
||||||
change.reviews = self.getPullReviews(change.project,
|
change.reviews = self.getPullReviews(pr_obj, change.project,
|
||||||
change.number)
|
change.number)
|
||||||
change.labels = change.pr.get('labels')
|
change.labels = change.pr.get('labels')
|
||||||
# ensure message is at least an empty string
|
# ensure message is at least an empty string
|
||||||
|
@ -1151,7 +1152,7 @@ class GithubConnection(BaseConnection):
|
||||||
|
|
||||||
log.debug('Got PR %s#%s', project_name, number)
|
log.debug('Got PR %s#%s', project_name, number)
|
||||||
self.log_rate_limit(self.log, github)
|
self.log_rate_limit(self.log, github)
|
||||||
return pr
|
return (pr, probj)
|
||||||
|
|
||||||
def canMerge(self, change, allow_needs):
|
def canMerge(self, change, allow_needs):
|
||||||
# NOTE: The mergeable call may get a false (null) while GitHub is
|
# NOTE: The mergeable call may get a false (null) while GitHub is
|
||||||
|
@ -1191,7 +1192,8 @@ class GithubConnection(BaseConnection):
|
||||||
raise Exception('Multiple pulls found with head sha %s' % sha)
|
raise Exception('Multiple pulls found with head sha %s' % sha)
|
||||||
if len(cached_pr_numbers) == 1:
|
if len(cached_pr_numbers) == 1:
|
||||||
for pr in cached_pr_numbers:
|
for pr in cached_pr_numbers:
|
||||||
return self.getPull(project, pr, log)
|
pr_body, pr_obj = self.getPull(project, pr, log)
|
||||||
|
return pr_body
|
||||||
|
|
||||||
pulls = []
|
pulls = []
|
||||||
project_name = project
|
project_name = project
|
||||||
|
@ -1216,10 +1218,11 @@ class GithubConnection(BaseConnection):
|
||||||
return None
|
return None
|
||||||
return pulls.pop()
|
return pulls.pop()
|
||||||
|
|
||||||
def getPullReviews(self, project, number):
|
def getPullReviews(self, pr_obj, project, number):
|
||||||
owner, proj = project.name.split('/')
|
# make a list out of the reviews so that we complete our
|
||||||
|
# API transaction
|
||||||
revs = self._getPullReviews(owner, proj, number)
|
revs = [review.as_dict() for review in pr_obj.reviews()]
|
||||||
|
self.log.debug('Got reviews for PR %s#%s', project, number)
|
||||||
|
|
||||||
permissions = {}
|
permissions = {}
|
||||||
reviews = {}
|
reviews = {}
|
||||||
|
@ -1325,17 +1328,6 @@ class GithubConnection(BaseConnection):
|
||||||
# we allow additional successful status contexts we don't care about.
|
# we allow additional successful status contexts we don't care about.
|
||||||
return required_contexts.issubset(successful)
|
return required_contexts.issubset(successful)
|
||||||
|
|
||||||
def _getPullReviews(self, owner, project, number):
|
|
||||||
# make a list out of the reviews so that we complete our
|
|
||||||
# API transaction
|
|
||||||
github = self.getGithubClient("%s/%s" % (owner, project))
|
|
||||||
reviews = [review.as_dict() for review in
|
|
||||||
github.pull_request(owner, project, number).reviews()]
|
|
||||||
|
|
||||||
self.log.debug('Got reviews for PR %s/%s#%s', owner, project, number)
|
|
||||||
self.log_rate_limit(self.log, github)
|
|
||||||
return reviews
|
|
||||||
|
|
||||||
def getUser(self, login, project):
|
def getUser(self, login, project):
|
||||||
return GithubUser(login, self, project)
|
return GithubUser(login, self, project)
|
||||||
|
|
||||||
|
|
|
@ -81,7 +81,7 @@ class GithubSource(BaseSource):
|
||||||
num = int(m.group(3))
|
num = int(m.group(3))
|
||||||
except ValueError:
|
except ValueError:
|
||||||
return None
|
return None
|
||||||
pull = self.connection.getPull('%s/%s' % (org, proj), int(num))
|
pull, pr_obj = self.connection.getPull('%s/%s' % (org, proj), int(num))
|
||||||
if not pull:
|
if not pull:
|
||||||
return None
|
return None
|
||||||
proj = pull.get('base').get('repo').get('full_name')
|
proj = pull.get('base').get('repo').get('full_name')
|
||||||
|
|
Loading…
Reference in New Issue