From 781b679648a6c26d2cf3c583dd3018f1f1815456 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 26 Mar 2018 13:08:49 +0200 Subject: [PATCH] [FIX] runbot_merge: de-f-string-ify --- runbot_merge/controllers.py | 38 +++++++++++----------- runbot_merge/github.py | 24 +++++++------- runbot_merge/models/pull_requests.py | 20 ++++++------ runbot_merge/tests/fake_github/__init__.py | 21 ++++++++---- runbot_merge/tests/test_basic.py | 12 +++---- runbot_merge/tests/test_multirepo.py | 16 ++++----- 6 files changed, 69 insertions(+), 62 deletions(-) diff --git a/runbot_merge/controllers.py b/runbot_merge/controllers.py index f955df2f..e29423aa 100644 --- a/runbot_merge/controllers.py +++ b/runbot_merge/controllers.py @@ -10,7 +10,7 @@ class MergebotController(Controller): def index(self): event = request.httprequest.headers['X-Github-Event'] - return EVENTS.get(event, lambda _: f"Unknown event {event}")(request.jsonrequest) + return EVENTS.get(event, lambda _: "Unknown event {}".format(event))(request.jsonrequest) def handle_pr(event): if event['action'] in [ @@ -23,7 +23,7 @@ def handle_pr(event): event['pull_request']['base']['repo']['full_name'], event['pull_request']['number'], ) - return f'Ignoring' + return 'Ignoring' env = request.env(user=1) pr = event['pull_request'] @@ -39,7 +39,7 @@ def handle_pr(event): # report actual errors to the webhooks listing thing on # github (not that we'd be looking at them but it'd be # useful for tests) - return f"Not configured to handle {r}" + return "Not configured to handle {}".format(r) # PRs to unmanaged branches are not necessarily abnormal and # we don't care @@ -67,7 +67,7 @@ def handle_pr(event): if not branch: pr = find(source_branch) pr.unlink() - return f'Retargeted {pr.id} to un-managed branch {b}, deleted' + return 'Retargeted {} to un-managed branch {}, deleted'.format(pr.id, b) # retargeting from un-managed => create if not source_branch: @@ -77,16 +77,16 @@ def handle_pr(event): if source_branch != branch: updates['target'] = branch.id if event['changes'].keys() & {'title', 'body'}: - updates['message'] = f"{pr['title'].strip()}\n\n{pr['body'].strip()}" + updates['message'] = "{}\n\n{}".format(pr['title'].strip(), pr['body'].strip()) if updates: pr_obj = find(source_branch) pr_obj.write(updates) - return f'Updated {pr_obj.id}' - return f"Nothing to update ({event['changes'].keys()})" + return 'Updated {}'.format(pr_obj.id) + return "Nothing to update ({})".format(event['changes'].keys()) if not branch: _logger.info("Ignoring PR for un-managed branch %s:%s", r, b) - return f"Not set up to care about {r}:{b}" + return "Not set up to care about {}:{}".format(r, b) author_name = pr['user']['login'] author = env['res.partner'].search([('github_login', '=', author_name)], limit=1) @@ -109,24 +109,24 @@ def handle_pr(event): 'repository': repo.id, 'head': pr['head']['sha'], 'squash': pr['commits'] == 1, - 'message': f'{title}\n\n{body}', + 'message': '{}\n\n{}'.format(title, body), }) - return f"Tracking PR as {pr_obj.id}" + return "Tracking PR as {}".format(pr_obj.id) pr_obj = find(branch) if not pr_obj: _logger.warn("webhook %s on unknown PR %s:%s", event['action'], repo.name, pr['number']) - return f"Unknown PR {repo.name}:{pr['number']}" + return "Unknown PR {}:{}".format(repo.name, pr['number']) if event['action'] == 'synchronize': if pr_obj.head == pr['head']['sha']: - return f'No update to pr head' + return 'No update to pr head' if pr_obj.state in ('closed', 'merged'): pr_obj.repository.github().comment( - pr_obj.number, f"This pull request is closed, ignoring the update to {pr['head']['sha']}") + pr_obj.number, "This pull request is closed, ignoring the update to {}".format(pr['head']['sha'])) # actually still update the head of closed (but not merged) PRs if pr_obj.state == 'merged': - return f'Ignoring update to {pr_obj.id}' + return 'Ignoring update to {}'.format(pr_obj.id) if pr_obj.state == 'validated': pr_obj.state = 'opened' @@ -145,19 +145,19 @@ def handle_pr(event): # TODO: should we update squash as well? What of explicit squash commands? pr_obj.head = pr['head']['sha'] - return f'Updated {pr_obj.id} to {pr_obj.head}' + return 'Updated {} to {}'.format(pr_obj.id, pr_obj.head) # don't marked merged PRs as closed (!!!) if event['action'] == 'closed' and pr_obj.state != 'merged': pr_obj.state = 'closed' - return f'Closed {pr_obj.id}' + return 'Closed {}'.format(pr_obj.id) if event['action'] == 'reopened' and pr_obj.state == 'closed': pr_obj.state = 'opened' - return f'Reopened {pr_obj.id}' + return 'Reopened {}'.format(pr_obj.id) _logger.info("Ignoring event %s on PR %s", event['action'], pr['number']) - return f"Not handling {event['action']} yet" + return "Not handling {} yet".format(event['action']) def handle_status(event): _logger.info( @@ -197,7 +197,7 @@ def handle_comment(event): return pr._parse_commands(partner, event['comment']['body']) def handle_ping(event): - print(f"Got ping! {event['zen']}") + print("Got ping! {}".format(event['zen'])) return "pong" EVENTS = { diff --git a/runbot_merge/github.py b/runbot_merge/github.py index 04aeaae7..624d27e3 100644 --- a/runbot_merge/github.py +++ b/runbot_merge/github.py @@ -14,7 +14,7 @@ class GH(object): self._url = 'https://api.github.com' self._repo = repo session = self._session = requests.Session() - session.headers['Authorization'] = f'token {token}' + session.headers['Authorization'] = 'token {}'.format(token) def __call__(self, method, path, json=None, check=True): """ @@ -22,7 +22,7 @@ class GH(object): """ r = self._session.request( method, - f'{self._url}/repos/{self._repo}/{path}', + '{}/repos/{}/{}'.format(self._url, self._repo, path), json=json ) if check: @@ -34,31 +34,31 @@ class GH(object): return r def head(self, branch): - d = self('get', f'git/refs/heads/{branch}').json() + d = self('get', 'git/refs/heads/{}'.format(branch)).json() - assert d['ref'] == f'refs/heads/{branch}' + assert d['ref'] == 'refs/heads/{}'.format(branch) assert d['object']['type'] == 'commit' return d['object']['sha'] def commit(self, sha): - return self('GET', f'git/commits/{sha}').json() + return self('GET', 'git/commits/{}'.format(sha)).json() def comment(self, pr, message): - self('POST', f'issues/{pr}/comments', json={'body': message}) + self('POST', 'issues/{}/comments'.format(pr), json={'body': message}) def close(self, pr, message): self.comment(pr, message) - self('PATCH', f'pulls/{pr}', json={'state': 'closed'}) + self('PATCH', 'pulls/{}'.format(pr), json={'state': 'closed'}) def fast_forward(self, branch, sha): try: - self('patch', f'git/refs/heads/{branch}', json={'sha': sha}) + self('patch', 'git/refs/heads/{}'.format(branch), json={'sha': sha}) except requests.HTTPError: raise exceptions.FastForwardError() def set_ref(self, branch, sha): # force-update ref - r = self('patch', f'git/refs/heads/{branch}', json={ + r = self('patch', 'git/refs/heads/{}'.format(branch), json={ 'sha': sha, 'force': True, }, check=False) @@ -68,7 +68,7 @@ class GH(object): if r.status_code == 404: # fallback: create ref r = self('post', 'git/refs', json={ - 'ref': f'refs/heads/{branch}', + 'ref': 'refs/heads/{}'.format(branch), 'sha': sha, }, check=False) if r.status_code == 201: @@ -102,7 +102,7 @@ class GH(object): cursor = None owner, name = self._repo.split('/') while True: - response = self._session.post(f'{self._url}/graphql', json={ + response = self._session.post('{}/graphql'.format(self._url), json={ 'query': PR_QUERY, 'variables': { 'owner': owner, @@ -117,7 +117,7 @@ class GH(object): author = into(pr, 'author.login') or into(pr, 'headRepositoryOwner.login') source = into(pr, 'headRepositoryOwner.login') or into(pr, 'author.login') - label = source and f"{source}:{pr['headRefName']}" + label = source and "{}:{}".format(source, pr['headRefName']) yield { 'number': pr['number'], 'title': pr['title'], diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 59b7aa64..e511b956 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -94,7 +94,7 @@ class Project(models.Model): prs.write({'state': 'merged'}) for pr in prs: # FIXME: this is the staging head rather than the actual merge commit for the PR - gh[pr.repository.name].close(pr.number, f'Merged in {staging_heads[pr.repository.name]}') + gh[pr.repository.name].close(pr.number, 'Merged in {}'.format(staging_heads[pr.repository.name])) finally: staging.batch_ids.unlink() staging.unlink() @@ -153,7 +153,7 @@ class Project(models.Model): gh = it['gh'] = repo.github() it['head'] = gh.head(branch.name) # create tmp staging branch - gh.set_ref(f'tmp.{branch.name}', it['head']) + gh.set_ref('tmp.{}'.format(branch.name), it['head']) batch_limit = project.batch_limit for batch in batches: @@ -173,7 +173,7 @@ class Project(models.Model): }) # create staging branch from tmp for r, it in meta.items(): - it['gh'].set_ref(f'staging.{branch.name}', it['head']) + it['gh'].set_ref('staging.{}'.format(branch.name), it['head']) logger.info("Created staging %s", st) def is_timed_out(self, staging): @@ -204,7 +204,7 @@ class Project(models.Model): ]) } for i, pr in enumerate(gh.prs()): - message = f"{pr['title'].strip()}\n\n{pr['body'].strip()}" + message = "{}\n\n{}".format(pr['title'].strip(), pr['body'].strip()) existing = prs.get(pr['number']) target = pr['base']['ref'] if existing: @@ -407,7 +407,7 @@ class PullRequests(models.Model): commands = dict( ps - for m in re.findall(f'^{self.repository.project_id.github_prefix}:? (.*)$', comment, re.MULTILINE) + for m in re.findall('^{}:? (.*)$'.format(self.repository.project_id.github_prefix), comment, re.MULTILINE) for c in m.strip().split() for ps in [self._parse_command(c)] if ps is not None @@ -462,9 +462,9 @@ class PullRequests(models.Model): author.github_login, author.display_name, ) if ok: - applied.append(f'{command}({param})') + applied.append('{}({})'.format(command, param)) else: - ignored.append(f'{command}({param})') + ignored.append('{}({})'.format(command, param)) msg = [] if applied: msg.append('applied ' + ' '.join(applied)) @@ -616,7 +616,7 @@ class Stagings(models.Model): # single batch => the staging is an unredeemable failure if self.state != 'failure': # timed out, just mark all PRs (wheee) - self.fail(f'timed out (>{self.target.project_id.ci_timeout} minutes)') + self.fail('timed out (>{} minutes)'.format(self.target.project_id.ci_timeout)) return False # try inferring which PR failed and only mark that one @@ -694,7 +694,7 @@ class Batch(models.Model): author = commit['author'] try: - new_heads[pr] = gh.merge(pr.head, f'tmp.{pr.target.name}', msg, squash=pr.squash, author=author)['sha'] + new_heads[pr] = gh.merge(pr.head, 'tmp.{}'.format(pr.target.name), msg, squash=pr.squash, author=author)['sha'] except exceptions.MergeError: _logger.exception("Failed to merge %s:%s into staging branch", pr.repository.name, pr.number) pr.state = 'error' @@ -703,7 +703,7 @@ class Batch(models.Model): # reset other PRs for to_revert in new_heads.keys(): it = meta[to_revert.repository] - it['gh'].set_ref(f'tmp.{to_revert.target.name}', it['head']) + it['gh'].set_ref('tmp.{}'.format(to_revert.target.name), it['head']) return self.env['runbot_merge.batch'] diff --git a/runbot_merge/tests/fake_github/__init__.py b/runbot_merge/tests/fake_github/__init__.py index 3712500e..13cdbc2a 100644 --- a/runbot_merge/tests/fake_github/__init__.py +++ b/runbot_merge/tests/fake_github/__init__.py @@ -104,7 +104,7 @@ class Repo(object): def make_pr(self, title, body, target, ctid, user, label=None): assert 'heads/%s' % target in self.refs - return PR(self, title, body, target, ctid, user=user, label=label or f'{user}:{target}') + return PR(self, title, body, target, ctid, user=user, label=label or '{}:{}'.format(user, target)) def make_ref(self, name, commit, force=False): assert isinstance(self.objects[commit], Commit) @@ -190,7 +190,7 @@ class Repo(object): m = re.match(pattern, path) if m: return handler(self, request, **m.groupdict()) - return (404, {'message': f"No match for {request.method} {path}"}) + return (404, {'message': "No match for {} {}".format(request.method, path)}) def _read_ref(self, r, ref): obj = self.refs.get(ref) @@ -496,12 +496,19 @@ class Commit(object): def __str__(self): parents = '\n'.join('parent {p}' for p in self.parents) + '\n' - return f"""commit {self.id} -tree {self.tree} -{parents}author {self.author} -committer {self.committer} + return """commit {} +tree {} +{}author {} +committer {} -{self.message}""" +{}""".format( + self.id, + self.tree, + parents, + self.author, + self.committer, + self.message +) class Client(werkzeug.test.Client): def __init__(self, application, path): diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index adf5c253..b2826e76 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -614,7 +614,7 @@ class TestPRUpdate(object): prx.push(c2) assert pr.head == c2, "PR should still be updated in case it's reopened" assert prx.comments == [ - ('', f"This pull request is closed, ignoring the update to {c2}"), + ('', "This pull request is closed, ignoring the update to {}".format(c2)), ] def test_reopen_update(self, env, repo): @@ -756,8 +756,8 @@ class TestPRUpdate(object): assert pr.head == c, "PR should not be updated at all" assert prx.comments == [ ('reviewer', 'hansen r+'), - ('', f'Merged in {h}'), - ('', f"This pull request is closed, ignoring the update to {c2}"), + ('', 'Merged in {}'.format(h)), + ('', "This pull request is closed, ignoring the update to {}".format(c2)), ] def test_update_error(self, env, repo): """ Should cancel the staging & reset PR to approved @@ -816,13 +816,13 @@ class TestBatching(object): each tree is an update on the "current state" of the tree :param target: branch, both the base commit and the PR target """ - base = repo.commit(f'heads/{target}') + base = repo.commit('heads/{}'.format(target)) tree = dict(repo.objects[base.tree]) c = base.id for i, t in enumerate(trees): tree.update(t) - c = repo.make_commit(c, f'commit_{prefix}_{i:02}', None, tree=dict(tree)) - pr = repo.make_pr(f'title {prefix}', f'body {prefix}', target=target, ctid=c, user=user, label=f'{user}:{prefix}') + c = repo.make_commit(c, 'commit_{}_{:02}'.format(prefix, i), None, tree=dict(tree)) + pr = repo.make_pr('title {}'.format(prefix), 'body {}'.format(prefix), target=target, ctid=c, user=user, label='{}:{}'.format(user, prefix)) repo.post_status(c, 'success', 'ci/runbot') repo.post_status(c, 'success', 'legal/cla') pr.post_comment('hansen r+', 'reviewer') diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 839e7ff4..0d03ab35 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -55,15 +55,15 @@ def repo_c(gh, project): ]) def make_pr(repo, prefix, trees, target='master', user='user', label=None): - base = repo.commit(f'heads/{target}') + base = repo.commit('heads/{}'.format(target)) tree = dict(repo.objects[base.tree]) c = base.id for i, t in enumerate(trees): tree.update(t) - c = repo.make_commit(c, f'commit_{prefix}_{i:02}', None, + c = repo.make_commit(c, 'commit_{}_{:02}'.format(prefix, i), None, tree=dict(tree)) - pr = repo.make_pr(f'title {prefix}', f'body {prefix}', target=target, - ctid=c, user=user, label=label and f'{user}:{label}') + pr = repo.make_pr('title {}'.format(prefix), 'body {}'.format(prefix), target=target, + ctid=c, user=user, label=label and '{}:{}'.format(user, label)) repo.post_status(c, 'success', 'ci/runbot') repo.post_status(c, 'success', 'legal/cla') pr.post_comment('hansen r+', 'reviewer') @@ -281,8 +281,8 @@ def test_batching(env, project, repo_a, repo_b): repo_b.make_ref('heads/master', repo_b.make_commit(None, 'initial', None, tree={'b': 'b0'})) prs = [( - a and to_pr(env, make_pr(repo_a, f'A{i}', [{f'a{i}': f'a{i}'}], label=f'batch{i}')), - b and to_pr(env, make_pr(repo_b, f'B{i}', [{f'b{i}': f'b{i}'}], label=f'batch{i}')) + a and to_pr(env, make_pr(repo_a, 'A{}'.format(i), [{'a{}'.format(i): 'a{}'.format(i)}], label='batch{}'.format(i))), + b and to_pr(env, make_pr(repo_b, 'B{}'.format(i), [{'b{}'.format(i): 'b{}'.format(i)}], label='batch{}'.format(i))) ) for i, (a, b) in enumerate([(1, 1), (0, 1), (1, 1), (1, 1), (1, 0)]) ] @@ -310,8 +310,8 @@ def test_batching_split(env, repo_a, repo_b): repo_b.make_ref('heads/master', repo_b.make_commit(None, 'initial', None, tree={'b': 'b0'})) prs = [( - a and to_pr(env, make_pr(repo_a, f'A{i}', [{f'a{i}': f'a{i}'}], label=f'batch{i}')), - b and to_pr(env, make_pr(repo_b, f'B{i}', [{f'b{i}': f'b{i}'}], label=f'batch{i}')) + a and to_pr(env, make_pr(repo_a, 'A{}'.format(i), [{'a{}'.format(i): 'a{}'.format(i)}], label='batch{}'.format(i))), + b and to_pr(env, make_pr(repo_b, 'B{}'.format(i), [{'b{}'.format(i): 'b{}'.format(i)}], label='batch{}'.format(i))) ) for i, (a, b) in enumerate([(1, 1), (0, 1), (1, 1), (1, 1), (1, 0)]) ]