From 39a0d723afc855838bff85ab8751644e80a15be8 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 28 Mar 2018 16:43:48 +0200 Subject: [PATCH] [ADD] runbot_merge: tag PR to signify state changes --- runbot_merge/github.py | 13 ++ runbot_merge/models/pull_requests.py | 138 +++++++++++++++++++++ runbot_merge/tests/fake_github/__init__.py | 31 ++++- runbot_merge/tests/test_basic.py | 21 +++- 4 files changed, 197 insertions(+), 6 deletions(-) diff --git a/runbot_merge/github.py b/runbot_merge/github.py index 624d27e3..2c9dddc7 100644 --- a/runbot_merge/github.py +++ b/runbot_merge/github.py @@ -50,6 +50,19 @@ class GH(object): self.comment(pr, message) self('PATCH', 'pulls/{}'.format(pr), json={'state': 'closed'}) + def change_tags(self, pr, from_, to_): + to_add, to_remove = to_ - from_, from_ - to_ + for t in to_remove: + r = self('DELETE', 'issues/{}/labels/{}'.format(pr, t), check=False) + r.raise_for_status() + # successful deletion or attempt to delete a tag which isn't there + # is fine, otherwise trigger an error + if r.status_code not in (200, 404): + r.raise_for_status() + + if to_add: + self('POST', 'issues/{}/labels'.format(pr), json=list(to_add)) + def fast_forward(self, branch, sha): try: self('patch', 'git/refs/heads/{}'.format(branch), json={'sha': sha}) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 86836875..a05251d4 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -177,8 +177,53 @@ class Project(models.Model): # create staging branch from tmp for r, it in meta.items(): it['gh'].set_ref('staging.{}'.format(branch.name), it['head']) + + # creating the staging doesn't trigger a write on the prs + # and thus the ->staging taggings, so do that by hand + Tagging = self.env['runbot_merge.pull_requests.tagging'] + for pr in st.mapped('batch_ids.prs'): + Tagging.create({ + 'pull_request': pr.number, + 'repository': pr.repository.id, + 'state_from': pr._tagstate, + 'state_to': 'staged', + }) + logger.info("Created staging %s", st) + Repos = self.env['runbot_merge.repository'] + ghs = {} + self.env.cr.execute(""" + SELECT + t.repository as repo_id, + t.pull_request as pr_number, + array_agg(t.id) as ids, + (array_agg(t.state_from ORDER BY t.id))[1] as state_from, + (array_agg(t.state_to ORDER BY t.id DESC))[1] as state_to + FROM runbot_merge_pull_requests_tagging t + GROUP BY t.repository, t.pull_request + """) + to_remove = [] + for repo_id, pr, ids, from_, to_ in self.env.cr.fetchall(): + repo = Repos.browse(repo_id) + from_tags = _TAGS[from_ or False] + to_tags = _TAGS[to_ or False] + + gh = ghs.get(repo) + if not gh: + gh = ghs[repo] = repo.github() + + try: + gh.change_tags(pr, from_tags, to_tags) + except Exception: + _logger.exception( + "Error while trying to change the tags of %s:%s from %s to %s", + repo.name, pr, from_tags, to_tags, + ) + else: + to_remove.extend(ids) + self.env['runbot_merge.pull_requests.tagging'].browse(to_remove).unlink() + def is_timed_out(self, staging): return fields.Datetime.from_string(staging.staged_at) + datetime.timedelta(minutes=self.ci_timeout) < datetime.datetime.now() @@ -505,6 +550,99 @@ class PullRequests(models.Model): self._cr, 'runbot_merge_unique_pr_per_target', self._table, ['number', 'target', 'repository']) return res + @property + def _tagstate(self): + if self.state == 'ready' and self.staging_id.heads: + return 'staged' + return self.state + + @api.model + def create(self, vals): + pr = super().create(vals) + self.env['runbot_merge.pull_requests.tagging'].create({ + 'pull_request': pr.number, + 'repository': pr.repository.id, + 'state_from': False, + 'state_to': pr._tagstate, + }) + return pr + + @api.multi + def write(self, vals): + oldstate = { pr: pr._tagstate for pr in self } + w = super().write(vals) + for pr in self: + before, after = oldstate[pr], pr._tagstate + if after != before: + self.env['runbot_merge.pull_requests.tagging'].create({ + 'pull_request': pr.number, + 'repository': pr.repository.id, + 'state_from': oldstate[pr], + 'state_to': pr._tagstate, + }) + return w + + @api.multi + def unlink(self): + for pr in self: + self.env['runbot_merge.pull_requests.tagging'].create({ + 'pull_request': pr.number, + 'repository': pr.repository.id, + 'state_from': pr._tagstate, + 'state_to': False, + }) + return super().unlink() + + +_TAGS = { + False: set(), + 'opened': {'seen 🙂'}, +} +_TAGS['validated'] = _TAGS['opened'] | {'CI 🤖'} +_TAGS['approved'] = _TAGS['opened'] | {'r+ 👌'} +_TAGS['ready'] = _TAGS['validated'] | _TAGS['approved'] +_TAGS['staged'] = _TAGS['ready'] | {'merging 👷'} +_TAGS['merged'] = _TAGS['ready'] | {'merged 🎉'} +_TAGS['error'] = _TAGS['opened'] | {'error 🙅'} +_TAGS['closed'] = _TAGS['opened'] | {'closed 💔'} + +class Tagging(models.Model): + """ + Queue of tag changes to make on PRs. + + Several PR state changes are driven by webhooks, webhooks should return + quickly, performing calls to the Github API would *probably* get in the + way of that. Instead, queue tagging changes into this table whose + execution can be cron-driven. + """ + _name = 'runbot_merge.pull_requests.tagging' + + repository = fields.Many2one('pull_request.repository', required=True) + # store the PR number (not id) as we need a Tagging for PR objects + # being deleted (retargeted to non-managed branches) + pull_request = fields.Integer() + + state_from = fields.Selection([ + ('opened', 'Opened'), + ('closed', 'Closed'), + ('validated', 'Validated'), + ('approved', 'Approved'), + ('ready', 'Ready'), + ('staged', 'Staged'), + ('merged', 'Merged'), + ('error', 'Error'), + ]) + state_to = fields.Selection([ + ('opened', 'Opened'), + ('closed', 'Closed'), + ('validated', 'Validated'), + ('approved', 'Approved'), + ('ready', 'Ready'), + ('staged', 'Staged'), + ('merged', 'Merged'), + ('error', 'Error'), + ]) + class Commit(models.Model): """Represents a commit onto which statuses might be posted, independent of everything else as commits can be created by diff --git a/runbot_merge/tests/fake_github/__init__.py b/runbot_merge/tests/fake_github/__init__.py index f4eac4ba..ba416548 100644 --- a/runbot_merge/tests/fake_github/__init__.py +++ b/runbot_merge/tests/fake_github/__init__.py @@ -1,11 +1,11 @@ import collections import io -import itertools import json import logging import re import responses +import werkzeug.urls import werkzeug.test import werkzeug.wrappers @@ -333,6 +333,31 @@ class Repo(object): return (200, {}) + def _add_labels(self, r, number): + try: + pr = self.issues[int(number)] + except KeyError: + return (404, None) + + pr.labels.update(json.loads(r.body)) + + return (200, {}) + + def _remove_label(self, r, number, label): + print('remove_label', number, label) + try: + pr = self.issues[int(number)] + except KeyError: + return (404, None) + + print(pr, pr.labels) + try: + pr.labels.remove(werkzeug.urls.url_unquote(label)) + except KeyError: + return (404, None) + else: + return (200, {}) + def _do_merge(self, r): body = json.loads(r.body) # {base, head, commit_message} if not body.get('commit_message'): @@ -394,6 +419,9 @@ class Repo(object): ('POST', r'merges', _do_merge), ('PATCH', r'pulls/(?P\d+)', _edit_pr), + + ('POST', r'issues/(?P\d+)/labels', _add_labels), + ('DELETE', r'issues/(?P\d+)/labels/(?P