From 0afe8797f4df519c60fd9e13a90e1446643b1d7f Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 22 Jun 2018 10:55:44 +0200 Subject: [PATCH] [IMP] runbot_merge: avoid unnecessary fetches * avoid fetching PRs for un-managed branches if we know up-front * avoid processing comments with no commands (avoids fetching the corresponding PR which we know nothing about yet and which may or may not be for a managed branch) --- runbot_merge/controllers/__init__.py | 29 +++++++++-------- runbot_merge/models/pull_requests.py | 48 +++++++++++++++++++++------- 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/runbot_merge/controllers/__init__.py b/runbot_merge/controllers/__init__.py index a59910aa..4950da1a 100644 --- a/runbot_merge/controllers/__init__.py +++ b/runbot_merge/controllers/__init__.py @@ -205,26 +205,26 @@ def handle_comment(env, event): if 'pull_request' not in event['issue']: return "issue comment, ignoring" - _logger.info( - 'comment: %s %s:%s "%s"', - event['sender']['login'], - event['repository']['full_name'], - event['issue']['number'], - event['comment']['body'], - ) + repo = event['repository']['full_name'] + issue = event['issue']['number'] + author = event['sender']['login'] + comment = event['comment']['body'] + _logger.info('comment: %s %s:%s "%s"', author, repo, issue, comment) - partner = env['res.partner'].search([('github_login', '=', event['sender']['login']),]) + partner = env['res.partner'].search([('github_login', '=', author), ]) if not partner: - _logger.info("ignoring comment from %s: not in system", event['sender']['login']) + _logger.info("ignoring comment from %s: not in system", author) return 'ignored' - pr = env['runbot_merge.pull_requests']._get_or_schedule( - event['repository']['full_name'], - event['issue']['number'], - ) + repository = env['runbot_merge.repository'].search([('name', '=', repo)]) + if not repository.project_id._find_commands(comment): + return "No commands, ignoring" + + pr = env['runbot_merge.pull_requests']._get_or_schedule(repo, issue) if not pr: return "Unknown PR, scheduling fetch" - return pr._parse_commands(partner, event['comment']['body']) + + return pr._parse_commands(partner, comment) def handle_review(env, event): partner = env['res.partner'].search([('github_login', '=', event['review']['user']['login'])]) @@ -235,6 +235,7 @@ def handle_review(env, event): pr = env['runbot_merge.pull_requests']._get_or_schedule( event['repository']['full_name'], event['pull_request']['number'], + event['pull_request']['base']['ref'] ) if not pr: return "Unknown PR, scheduling fetch" diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 14eaea8e..13961e97 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -243,17 +243,26 @@ class Project(models.Model): if not f: return - repo = self.env['runbot_merge.repository'].search([('name', '=', f.repository)]) - if repo: - repo._load_pr(f.number) - else: - _logger.warn("Fetch job for unknown repository %s, disabling & skipping", f.repository) + f.repository._load_pr(f.number) # commit after each fetched PR f.active = False if commit: self.env.cr.commit() + def _find_commands(self, comment): + return re.findall( + '^{}:? (.*)$'.format(self.github_prefix), + comment, re.MULTILINE) + + def _has_branch(self, name): + self.env.cr.execute(""" + SELECT 1 FROM runbot_merge_branch + WHERE project_id = %s AND name = %s + LIMIT 1 + """, (self.id, name)) + return bool(self.env.cr.rowcount) + class Repository(models.Model): _name = 'runbot_merge.repository' @@ -274,6 +283,11 @@ class Repository(models.Model): # fetch PR object and handle as *opened* issue, pr = gh.pr(number) + + if not self.project_id._has_branch(pr['base']['ref']): + _logger.info("Tasked with loading PR %d for un-managed branch %s, ignoring", pr['number'], pr['base']['ref']) + return + controllers.handle_pr(self.env, { 'action': 'opened', 'pull_request': pr, @@ -382,18 +396,28 @@ class PullRequests(models.Model): for r in self: r.batch_id = r.batch_ids.filtered(lambda b: b.active)[:1] - def _get_or_schedule(self, repo_name, number): + def _get_or_schedule(self, repo_name, number, target=None): + repo = self.env['runbot_merge.repository'].search([('name', '=', repo_name)]) + if not repo: + return + + if target and not repo.project_id._has_branch(target): + return + pr = self.search([ - ('repository.name', '=', repo_name), + ('repository', '=', repo.id), ('number', '=', number,) ]) if pr: return pr Fetch = self.env['runbot_merge.fetch_job'] - if Fetch.search([('repository', '=', repo_name), ('number', '=', number)]): + if Fetch.search([('repository', '=', repo.id), ('number', '=', number)]): return - Fetch.create({'repository': repo_name, 'number': number}) + Fetch.create({ + 'repository': repo.id, + 'number': number, + }) def _parse_command(self, commandstring): m = re.match(r'(\w+)(?:([+-])|=(.*))?', commandstring) @@ -452,7 +476,7 @@ class PullRequests(models.Model): commands = dict( ps - for m in re.findall('^{}:? (.*)$'.format(self.repository.project_id.github_prefix), comment, re.MULTILINE) + for m in self.repository.project_id._find_commands(comment) for c in m.strip().split() for ps in [self._parse_command(c)] if ps is not None @@ -901,5 +925,5 @@ class FetchJob(models.Model): _name = 'runbot_merge.fetch_job' active = fields.Boolean(default=True) - repository = fields.Char(index=True) - number = fields.Integer(index=True) + repository = fields.Many2one('runbot_merge.repository', index=True, required=True) + number = fields.Integer(index=True, required=True)