Skip to content

Commit

Permalink
Auto merge of barosl#87 - servo:treeclosed, r=edunham
Browse files Browse the repository at this point in the history 
Implement tree closure

fixes barosl#85 (also drive-by fixes barosl#86)

This is implemented as `@bors-servo treeclosed=<number>`, which closes the tree for PRs below a priority value. `@bors-servo treeclosed-` (or `@bors-servo treeclosed=-1`) restores the tree.

cc @jdm @cgwalters

r? @edunham

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/homu/87)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo authored Jan 26, 2017
2 parents d59f9f5 + 35bdcf6 commit a7355eb
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 29 deletions.
2 changes: 1 addition & 1 deletion homu/html/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ <h2>Repositories</h2>

<ul class="repos">
{% for repo in repos %}
<li><a href="queue/{{repo}}">{{repo}}</a></li>
<li><a href="queue/{{repo.repo_label}}">{{repo.repo_label}}</a> {% if repo.treeclosed >= 0 %} [TREE CLOSED] {% endif %}</li>
{% endfor %}
</ul>

Expand Down
7 changes: 4 additions & 3 deletions homu/html/queue.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<html>
<head>
<meta charset="utf-8">
<title>Homu queue - {{repo_label}}</title>
<title>Homu queue - {{repo_label}} {% if treeclosed %} [TREE CLOSED] {% endif %}</title>
<style>
* { font-family: sans-serif; }
h1 { font-size: 20px; }
Expand All @@ -13,6 +13,7 @@
td, th { border: 2px solid white; padding: 5px; font-size: 13px; }
tr:nth-child(even) { background: #ddd; }

.treeclosed { color: grey }
.success { background-color: #80C0F0; }
.failure, .error { background-color: #F08080; }
.pending { background-color: #F0DE57; }
Expand All @@ -30,7 +31,7 @@
</style>
</head>
<body>
<h1>Homu queue - {% if repo_url %}<a href="{{repo_url}}" target="_blank">{{repo_label}}</a>{% else %}{{repo_label}}{% endif %}</h1>
<h1>Homu queue - {% if repo_url %}<a href="{{repo_url}}" target="_blank">{{repo_label}}</a>{% else %}{{repo_label}}{% endif %} {% if treeclosed %} [TREE CLOSED below priority {{treeclosed}}] {% endif %}</h2>

<p>
<button type="button" id="rollup">Create a rollup</button>
Expand Down Expand Up @@ -67,7 +68,7 @@ <h1>Homu queue - {% if repo_url %}<a href="{{repo_url}}" target="_blank">{{repo_

<tbody>
{% for state in states %}
<tr>
<tr class="{{state.greyed}}">
<td class="hide">{{loop.index}}</td>
<td><input type="checkbox" data-num="{{state.num}}"></td>
{% if multiple %}
Expand Down
103 changes: 82 additions & 21 deletions homu/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,33 @@ def db_query(db, *args):
db.execute(*args)


class Repository:
treeclosed = -1
gh = None
label = None
db = None

def __init__(self, gh, repo_label, db):
self.gh = gh
self.repo_label = repo_label
self.db = db
db_query(db, 'SELECT treeclosed FROM repos WHERE repo = ?', [repo_label])
row = db.fetchone()
if row:
self.treeclosed = row[0]
else:
self.treeclosed = -1

def update_treeclosed(self, value):
self.treeclosed = value
db_query(self.db, 'DELETE FROM repos where repo = ?', [self.repo_label])
if value > 0:
db_query(self.db, 'INSERT INTO repos (repo, treeclosed) VALUES (?, ?)', [self.repo_label, value])

def __lt__(self, other):
return self.gh < other.gh


class PullReqState:
num = 0
priority = 0
Expand Down Expand Up @@ -185,9 +212,9 @@ def build_res_summary(self):
for builder, data in self.build_res.items())

def get_repo(self):
repo = self.repos[self.repo_label]
repo = self.repos[self.repo_label].gh
if not repo:
self.repos[self.repo_label] = repo = self.gh.repository(self.owner, self.name)
self.repos[self.repo_label].gh = repo = self.gh.repository(self.owner, self.name)

assert repo.owner.login == self.owner
assert repo.name == self.name
Expand Down Expand Up @@ -231,6 +258,13 @@ def fake_merge(self, repo_cfg):
title = merged_prefix + title
issue.edit(title=title)

def change_treeclosed(self, value):
self.repos[self.repo_label].update_treeclosed(value)

def blocked_by_closed_tree(self):
treeclosed = self.repos[self.repo_label].treeclosed
return treeclosed if self.priority < treeclosed else None


def sha_cmp(short, full):
return len(short) >= 4 and short == full[:len(short)]
Expand All @@ -247,7 +281,15 @@ class AuthState(IntEnum):
NONE = 1


def verify_auth(username, repo_cfg, state, auth, realtime):
def verify_auth(username, repo_cfg, state, auth, realtime, my_username):
# In some cases (e.g. non-fully-qualified r+) we recursively talk to ourself
# via a hidden markdown comment in the message. This is so that
# when re-synchronizing after shutdown we can parse these comments
# and still know the SHA for the approval.
#
# So comments from self should always be allowed
if username == my_username:
return True
is_reviewer = False
auth_collaborators = repo_cfg.get('auth_collaborators', False)
if auth_collaborators:
Expand Down Expand Up @@ -284,9 +326,6 @@ def verify_auth(username, repo_cfg, state, auth, realtime):


def parse_commands(body, username, repo_cfg, state, my_username, db, states, *, realtime=False, sha=''):
# Skip parsing notifications that we created
if username == my_username:
return False

state_changed = False

Expand All @@ -295,9 +334,8 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, *,
state.add_comment(":cake: {}\n\n![]({})".format(random.choice(PORTAL_TURRET_DIALOG), PORTAL_TURRET_IMAGE))
for i, word in reversed(list(enumerate(words))):
found = True

if word == 'r+' or word.startswith('r='):
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime, my_username):
continue

if not sha and i + 1 < len(words):
Expand Down Expand Up @@ -367,17 +405,20 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, *,
state.add_comment(':scream_cat: {} Please try again with `{:.7}`.'.format(msg, state.head_sha))
else:
state.add_comment(':pushpin: Commit {:.7} has been approved by `{}`\n\n<!-- @{} r={} {} -->'.format(state.head_sha, approver, my_username, approver, state.head_sha))
treeclosed = state.blocked_by_closed_tree()
if treeclosed:
state.add_comment(':evergreen_tree: The tree is currently closed for pull requests below priority {}, this pull request will be tested once the tree is reopened'.format(treeclosed))

elif word == 'r-':
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime, my_username):
continue

state.approved_by = ''

state.save()

elif word.startswith('p='):
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime, my_username):
continue
try:
state.priority = int(word[len('p='):])
Expand All @@ -387,7 +428,7 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, *,
state.save()

elif word.startswith('delegate='):
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime, my_username):
continue

state.delegate = word[len('delegate='):]
Expand All @@ -398,13 +439,13 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, *,

elif word == 'delegate-':
# TODO: why is this a TRY?
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime, my_username):
continue
state.delegate = ''
state.save()

elif word == 'delegate+':
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime, my_username):
continue

state.delegate = state.get_repo().pull_request(state.num).user.login
Expand All @@ -414,12 +455,12 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, *,
state.add_comment(':v: @{} can now approve this pull request'.format(state.delegate))

elif word == 'retry' and realtime:
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime, my_username):
continue
state.set_status('')

elif word in ['try', 'try-'] and realtime:
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime, my_username):
continue
state.try_ = word == 'try'

Expand All @@ -429,14 +470,14 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, *,
state.save()

elif word in ['rollup', 'rollup-']:
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime, my_username):
continue
state.rollup = word == 'rollup'

state.save()

elif word == 'force' and realtime:
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime, my_username):
continue
if 'buildbot' in repo_cfg:
with buildbot_sess(repo_cfg) as sess:
Expand All @@ -460,14 +501,28 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, *,
state.add_comment(':bomb: Buildbot returned an error: `{}`'.format(err))

elif word == 'clean' and realtime:
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime):
if not verify_auth(username, repo_cfg, state, AuthState.TRY, realtime, my_username):
continue
state.merge_sha = ''
state.init_build_res([])

state.save()
elif word == 'hello?' or word == 'ping':
state.add_comment(":sleepy: I'm awake I'm awake")
elif word.startswith('treeclosed='):
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime, my_username):
continue
try:
treeclosed = int(word[len('treeclosed='):])
state.change_treeclosed(treeclosed)
except ValueError:
pass
state.save()
elif word == 'treeclosed-':
if not verify_auth(username, repo_cfg, state, AuthState.REVIEWER, realtime, my_username):
continue
state.change_treeclosed(-1)
state.save()
else:
found = False

Expand Down Expand Up @@ -945,6 +1000,8 @@ def process_queue(states, repos, repo_cfgs, logger, buildbot_slots, db, git_cfg)
repo_states = sorted(states[repo_label].values())

for state in repo_states:
if state.priority < repo.treeclosed:
break
if state.status == 'pending' and not state.try_:
break

Expand Down Expand Up @@ -1052,7 +1109,7 @@ def synchronize(repo_label, repo_cfg, logger, gh, states, repos, db, mergeable_q
}

states[repo_label] = {}
repos[repo_label] = repo
repos[repo_label] = Repository(repo, repo_label, db)

for pull in repo.iter_pulls(state='open'):
db_query(db, 'SELECT status FROM pull WHERE repo = ? AND num = ?', [repo_label, pull.number])
Expand Down Expand Up @@ -1204,13 +1261,17 @@ def main():
mergeable INTEGER NOT NULL,
UNIQUE (repo, num)
)''')

db_query(db, '''CREATE TABLE IF NOT EXISTS repos (
repo TEXT NOT NULL,
treeclosed INTEGER NOT NULL,
UNIQUE (repo)
)''')
for repo_label, repo_cfg in cfg['repo'].items():
repo_cfgs[repo_label] = repo_cfg
repo_labels[repo_cfg['owner'], repo_cfg['name']] = repo_label

repo_states = {}
repos[repo_label] = None
repos[repo_label] = Repository(None, repo_label, db)

db_query(db, 'SELECT num, head_sha, status, title, body, head_ref, base_ref, assignee, approved_by, priority, try_, rollup, delegate, merge_sha FROM pull WHERE repo = ?', [repo_label])
for num, head_sha, status, title, body, head_ref, base_ref, assignee, approved_by, priority, try_, rollup, delegate, merge_sha in db.fetchall():
Expand Down
21 changes: 17 additions & 4 deletions homu/server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def find_state(sha):


def get_repo(repo_label, repo_cfg):
repo = g.repos[repo_label]
repo = g.repos[repo_label].gh
if not repo:
g.repos[repo_label] = repo = g.gh.repository(repo_cfg['owner'], repo_cfg['name'])

Expand All @@ -48,7 +48,7 @@ def get_repo(repo_label, repo_cfg):

@get('/')
def index():
return g.tpls['index'].render(repos=sorted(g.repos))
return g.tpls['index'].render(repos=[g.repos[label] for label in sorted(g.repos)])


@get('/queue/<repo_label:path>')
Expand All @@ -57,13 +57,16 @@ def queue(repo_label):

lazy_debug(logger, lambda: 'repo_label: {}'.format(repo_label))

single_repo_closed = None
if repo_label == 'all':
labels = g.repos.keys()
multiple = True
repo_url = None
else:
labels = repo_label.split('+')
multiple = len(labels) > 1
if repo_label in g.repos and g.repos[repo_label].treeclosed >= 0:
single_repo_closed = g.repos[repo_label].treeclosed
repo_url = 'https://github.com/{}/{}'.format(
g.cfg['repo'][repo_label]['owner'],
g.cfg['repo'][repo_label]['name'])
Expand All @@ -76,12 +79,20 @@ def queue(repo_label):
abort(404, 'No such repository: {}'.format(label))

pull_states = sorted(states)

rows = []
for state in pull_states:
treeclosed = single_repo_closed or state.priority < g.repos[state.repo_label].treeclosed
status_ext = ''

if state.try_:
status_ext += ' (try)'

if treeclosed:
status_ext += ' [TREE CLOSED]'

rows.append({
'status': state.get_status(),
'status_ext': ' (try)' if state.try_ else '',
'status_ext': status_ext,
'priority': 'rollup' if state.rollup else state.priority,
'url': 'https://github.com/{}/{}/pull/{}'.format(state.owner, state.name, state.num),
'num': state.num,
Expand All @@ -92,11 +103,13 @@ def queue(repo_label):
'assignee': state.assignee,
'repo_label': state.repo_label,
'repo_url': 'https://github.com/{}/{}'.format(state.owner, state.name),
'greyed': "treeclosed" if treeclosed else "",
})

return g.tpls['queue'].render(
repo_url=repo_url,
repo_label=repo_label,
treeclosed=single_repo_closed,
states=rows,
oauth_client_id=g.cfg['github']['app_client_id'],
total=len(pull_states),
Expand Down

0 comments on commit a7355eb

Please sign in to comment.