gitlab bot: major refactoring of code review scan
this moves a bunch of code around and renames a bunch of things, so the diff for it is fairly huge, but it's the same general idea, just organized better. woo.
This commit is contained in:
parent
fca69b1994
commit
a0f76e4c0f
|
|
@ -15,13 +15,16 @@ class GitlabBot(object):
|
||||||
|
|
||||||
"""Bot for doing GitLab stuff. Might be used by the IRC bot, or daemons, or whatever."""
|
"""Bot for doing GitLab stuff. Might be used by the IRC bot, or daemons, or whatever."""
|
||||||
|
|
||||||
COUNTER_RESET_FORMAT = "Review counter reset."
|
REVIEWS_START_FORMAT = "Starting review process."
|
||||||
COUNTER_START_FORMAT = "Please obtain {0:d} reviews."
|
REVIEWS_RESET_FORMAT = "Review process reset."
|
||||||
NEW_REVIEWER_FORMAT = "{0:d} of {1:d} reviewed. Assigning to {2:s} to review."
|
REVIEWS_REVIEW_REGISTERED_FORMAT = "Review identified."
|
||||||
REVIEWS_DONE_FORMAT = "Reviews conducted. Assigning to {0:s} to approve."
|
REVIEWS_PROGRESS_FORMAT = "{0:d} reviews of {1:d} necessary to proceed."
|
||||||
|
REVIEWS_REVIEWS_COMPLETE = "Reviews complete."
|
||||||
|
NEW_REVIEWER_FORMAT = "Assigning to {0:s} to review merge request."
|
||||||
|
NEW_ACCEPTER_FORMAT = "Assigning to {0:s} to accept merge request."
|
||||||
|
|
||||||
NOTE_COMMENT = [
|
NOTE_COMMENT = [
|
||||||
"The computer is your friend.",
|
"The computer is your friend.",
|
||||||
"Thank you for complying.",
|
|
||||||
]
|
]
|
||||||
|
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
|
|
@ -30,66 +33,26 @@ class GitlabBot(object):
|
||||||
self.client = gitlab.Gitlab(config.url, private_token=config.token)
|
self.client = gitlab.Gitlab(config.url, private_token=config.token)
|
||||||
self.client.auth()
|
self.client.auth()
|
||||||
|
|
||||||
def random_counter_reset_message(self):
|
def random_reviews_start_message(self):
|
||||||
return "{0:s} {1:s} {2:s}".format(self.COUNTER_RESET_FORMAT, self.COUNTER_START_FORMAT,
|
return "{0:s} {1:s} {2:s}".format(self.REVIEWS_START_FORMAT, self.REVIEWS_PROGRESS_FORMAT,
|
||||||
random.choice(self.NOTE_COMMENT))
|
random.choice(self.NOTE_COMMENT))
|
||||||
|
|
||||||
|
def random_reviews_reset_message(self):
|
||||||
|
return "{0:s} {1:s} {2:s}".format(self.REVIEWS_RESET_FORMAT, self.REVIEWS_PROGRESS_FORMAT,
|
||||||
|
random.choice(self.NOTE_COMMENT))
|
||||||
|
|
||||||
|
def random_review_progress_message(self):
|
||||||
|
return "{0:s} {1:s} {2:s}".format(self.REVIEWS_REVIEW_REGISTERED_FORMAT, self.REVIEWS_PROGRESS_FORMAT,
|
||||||
|
random.choice(self.NOTE_COMMENT))
|
||||||
|
|
||||||
|
def random_reviews_complete_message(self):
|
||||||
|
return "{0:s} {1:s}".format(self.REVIEWS_REVIEWS_COMPLETE, random.choice(self.NOTE_COMMENT))
|
||||||
|
|
||||||
def random_new_reviewer_message(self):
|
def random_new_reviewer_message(self):
|
||||||
return "{0:s} {1:s}".format(self.NEW_REVIEWER_FORMAT, random.choice(self.NOTE_COMMENT))
|
return "{0:s} {1:s}".format(self.NEW_REVIEWER_FORMAT, random.choice(self.NOTE_COMMENT))
|
||||||
|
|
||||||
def random_reviews_done_message(self):
|
def random_reviews_done_message(self):
|
||||||
return "{0:s} {1:s}".format(self.REVIEWS_DONE_FORMAT, random.choice(self.NOTE_COMMENT))
|
return "{0:s} {1:s}".format(self.NEW_ACCEPTER_FORMAT, random.choice(self.NOTE_COMMENT))
|
||||||
|
|
||||||
def get_code_review_acceptance_count(self, merge_request):
|
|
||||||
"""Walk the provided merge request and determine the number of code review signoffs in the comments."""
|
|
||||||
if type(merge_request) != gitlab.objects.ProjectMergeRequest:
|
|
||||||
raise TypeError("merge_request must be a ProjectMergeRequest object")
|
|
||||||
|
|
||||||
approve_count = 0
|
|
||||||
approvers = list()
|
|
||||||
reset_found = False
|
|
||||||
send_reset = False
|
|
||||||
notes = sorted(self.client.project_mergerequest_notes.list(project_id=merge_request.project_id,
|
|
||||||
merge_request_id=merge_request.id,
|
|
||||||
all=True),
|
|
||||||
key=lambda x: x.id)
|
|
||||||
for note in notes:
|
|
||||||
if not note.system:
|
|
||||||
# note that we can't ignore note.system = True + merge_request.author, that might have been
|
|
||||||
# a new push or something, so we only ignore normal notes from the author
|
|
||||||
if note.author == merge_request.author:
|
|
||||||
log.debug("skipping note from the merge request author")
|
|
||||||
elif note.author.username == self.client.user.username:
|
|
||||||
log.debug("saw a message from myself, maybe i already nagged about the reset")
|
|
||||||
if note.body.find(self.COUNTER_RESET_FORMAT) >= 0:
|
|
||||||
log.debug("...yup")
|
|
||||||
send_reset = False
|
|
||||||
else:
|
|
||||||
log.debug("...nope")
|
|
||||||
else:
|
|
||||||
if note.body.find("LGTM") >= 0:
|
|
||||||
log.debug("merge request '%s', note '%s' has a LGTM", merge_request.title, note.id)
|
|
||||||
approve_count += 1
|
|
||||||
if reset_found:
|
|
||||||
log.debug("ignoring the previous reset, since we have a counter")
|
|
||||||
reset_found = False
|
|
||||||
send_reset = False
|
|
||||||
approvers.append(note.author.username)
|
|
||||||
log.debug("approval count set to %d", approve_count)
|
|
||||||
else:
|
|
||||||
log.debug("merge request '%s', note '%s' does not have a LGTM", merge_request.title, note.id)
|
|
||||||
else:
|
|
||||||
log.debug("merge request '%s', note '%s' is a system message", merge_request.title, note.id)
|
|
||||||
if re.match(r'Added \d+ commit', note.body):
|
|
||||||
log.debug("setting the counter to 0, '%s' looks like a push!", note.body)
|
|
||||||
approve_count = 0
|
|
||||||
approvers = list()
|
|
||||||
reset_found = True
|
|
||||||
send_reset = True
|
|
||||||
else:
|
|
||||||
log.debug("leaving the counter as it is, i don't think '%s' is a push", note.body)
|
|
||||||
|
|
||||||
return approve_count, approvers, send_reset
|
|
||||||
|
|
||||||
def scan_project_for_reviews(self, project):
|
def scan_project_for_reviews(self, project):
|
||||||
project_obj = self.client.projects.get(project.project_id)
|
project_obj = self.client.projects.get(project.project_id)
|
||||||
|
|
@ -99,29 +62,124 @@ class GitlabBot(object):
|
||||||
for merge_request in project_obj.mergerequests.list(state='opened'):
|
for merge_request in project_obj.mergerequests.list(state='opened'):
|
||||||
log.debug("scanning merge request '%s'", merge_request.title)
|
log.debug("scanning merge request '%s'", merge_request.title)
|
||||||
|
|
||||||
# check to see if the merge request needs a reviewer or a merge
|
# moved this method into here
|
||||||
approve_count, approvers, send_reset = self.get_code_review_acceptance_count(merge_request)
|
# # check to see if the merge request needs a reviewer or a merge
|
||||||
if approve_count < project.code_reviews_necessary:
|
# approve_count, approvers, send_reset = self.get_code_review_acceptance_count(merge_request)
|
||||||
|
|
||||||
|
request_state = _MergeRequestScanningState()
|
||||||
|
notes = sorted(self.client.project_mergerequest_notes.list(project_id=merge_request.project_id,
|
||||||
|
merge_request_id=merge_request.id,
|
||||||
|
all=True),
|
||||||
|
key=lambda x: x.id)
|
||||||
|
for note in notes:
|
||||||
|
if not note.system:
|
||||||
|
log.debug("merge request '%s', note '%s' is a normal message", merge_request.title, note.id)
|
||||||
|
# note that we can't ignore note.system = True + merge_request.author, that might have been
|
||||||
|
# a new push or something, so we only ignore normal notes from the author
|
||||||
|
if note.author == merge_request.author:
|
||||||
|
log.debug("skipping note from the merge request author")
|
||||||
|
elif note.author.username == self.client.user.username:
|
||||||
|
log.debug("saw a message from myself, i might have already sent a notice i'm sitting on")
|
||||||
|
if note.body.find(self.REVIEWS_RESET_FORMAT) >= 0 and request_state.unlogged_approval_reset:
|
||||||
|
log.debug("saw a reset message, unsetting the flag")
|
||||||
|
request_state.unlogged_approval_reset = False
|
||||||
|
elif note.body.find(self.REVIEWS_START_FORMAT) >= 0 and request_state.unlogged_review_start:
|
||||||
|
log.debug("saw a start message, unsetting the flag")
|
||||||
|
request_state.unlogged_review_start = False
|
||||||
|
elif note.body.find(self.REVIEWS_REVIEW_REGISTERED_FORMAT) >= 0 and request_state.unlogged_approval:
|
||||||
|
log.debug("saw a review log message, unsetting the flag")
|
||||||
|
request_state.unlogged_approval = False
|
||||||
|
elif note.body.find(self.REVIEWS_REVIEWS_COMPLETE) >= 0 and request_state.unlogged_review_complete:
|
||||||
|
log.debug("saw a review complete message, unsetting the flag")
|
||||||
|
request_state.unlogged_review_complete = False
|
||||||
|
else:
|
||||||
|
log.debug("nothing in particular relevant in '%s'", note.body)
|
||||||
|
else:
|
||||||
|
if note.body.find("LGTM") >= 0:
|
||||||
|
log.debug("merge request '%s', note '%s' has a LGTM", merge_request.title, note.id)
|
||||||
|
request_state.unlogged_approval = True
|
||||||
|
request_state.approver_list.append(note.author.username)
|
||||||
|
log.debug("approvers: %s", request_state.approver_list)
|
||||||
|
if len(request_state.approver_list) < project.code_reviews_necessary:
|
||||||
|
log.debug("not enough code reviews yet, setting needs_reviewer")
|
||||||
|
request_state.needs_reviewer = True
|
||||||
|
request_state.needs_accepter = False
|
||||||
|
else:
|
||||||
|
log.debug("enough code reviews, setting needs_accepter")
|
||||||
|
request_state.needs_accepter = True
|
||||||
|
request_state.needs_reviewer = False
|
||||||
|
request_state.unlogged_review_complete = True
|
||||||
|
else:
|
||||||
|
log.debug("merge request '%s', note '%s' does not have a LGTM", merge_request.title,
|
||||||
|
note.id)
|
||||||
|
else:
|
||||||
|
log.debug("merge request '%s', note '%s' is a system message", merge_request.title, note.id)
|
||||||
|
if re.match(r'Added \d+ commit', note.body):
|
||||||
|
log.debug("resetting approval list, '%s' looks like a push!", note.body)
|
||||||
|
request_state.approver_list.clear()
|
||||||
|
request_state.unlogged_approval_reset = True
|
||||||
|
else:
|
||||||
|
log.debug("leaving the approval list as it is, i don't think '%s' is a push", note.body)
|
||||||
|
|
||||||
|
# do some cleanup
|
||||||
|
excluded_review_candidates = request_state.approver_list + [merge_request.author.username]
|
||||||
|
review_candidates = [x for x in project.code_reviewers.split(',') if x not in excluded_review_candidates]
|
||||||
|
if merge_request.assignee:
|
||||||
|
if request_state.needs_reviewer and merge_request.assignee.username in review_candidates:
|
||||||
|
log.debug("unsetting the needs_reviewer flag, the request is already assigned to one")
|
||||||
|
request_state.needs_reviewer = False
|
||||||
|
|
||||||
|
excluded_accept_candidates = [merge_request.author.username]
|
||||||
|
accept_candidates = [x for x in project.code_review_final_merge_assignees.split(',') if x not in excluded_accept_candidates]
|
||||||
|
if merge_request.assignee:
|
||||||
|
if request_state.needs_accepter and merge_request.assignee.username in accept_candidates:
|
||||||
|
log.debug("unsetting the needs_accepter flag, the request is already assigned to one")
|
||||||
|
request_state.needs_accepter = False
|
||||||
|
|
||||||
|
log.debug("%s", request_state.__dict__)
|
||||||
|
|
||||||
|
# status message stuff
|
||||||
|
if request_state.unlogged_review_start:
|
||||||
|
log.info("sending message for start of reviews")
|
||||||
|
msg = {'body': self.random_reviews_start_message().format(len(request_state.approver_list),
|
||||||
|
project.code_reviews_necessary)}
|
||||||
|
self.client.project_mergerequest_notes.create(msg, project_id=project_obj.id,
|
||||||
|
merge_request_id=merge_request.id)
|
||||||
|
if request_state.unlogged_approval_reset:
|
||||||
|
log.info("sending message for review reset")
|
||||||
|
msg = {'body': self.random_reviews_reset_message().format(len(request_state.approver_list),
|
||||||
|
project.code_reviews_necessary)}
|
||||||
|
self.client.project_mergerequest_notes.create(msg, project_id=project_obj.id,
|
||||||
|
merge_request_id=merge_request.id)
|
||||||
|
if request_state.unlogged_approval:
|
||||||
|
log.info("sending message for code review progress")
|
||||||
|
msg = {'body': self.random_review_progress_message().format(len(request_state.approver_list),
|
||||||
|
project.code_reviews_necessary)}
|
||||||
|
self.client.project_mergerequest_notes.create(msg, project_id=project_obj.id,
|
||||||
|
merge_request_id=merge_request.id)
|
||||||
|
if request_state.unlogged_review_complete:
|
||||||
|
log.info("sending message for code review complete")
|
||||||
|
msg = {'body': self.random_reviews_complete_message()}
|
||||||
|
self.client.project_mergerequest_notes.create(msg, project_id=project_obj.id,
|
||||||
|
merge_request_id=merge_request.id)
|
||||||
|
|
||||||
|
# if there's a reviewer necessary, assign the merge request
|
||||||
|
if len(request_state.approver_list) < project.code_reviews_necessary and request_state.needs_reviewer:
|
||||||
log.debug("%s needs a code review", merge_request.title)
|
log.debug("%s needs a code review", merge_request.title)
|
||||||
if merge_request.assignee is not None:
|
if merge_request.assignee is not None:
|
||||||
log.debug("%s currently assigned to %s", merge_request.title, merge_request.assignee.username)
|
log.debug("%s currently assigned to %s", merge_request.title, merge_request.assignee.username)
|
||||||
excluded_candidates = approvers + [merge_request.author.username]
|
|
||||||
candidates = [x for x in project.code_reviewers.split(',') if x not in excluded_candidates]
|
|
||||||
|
|
||||||
# if the current assignee is in candidates, then we don't need to do anything
|
if merge_request.assignee is None or merge_request.assignee.username not in review_candidates:
|
||||||
if merge_request.assignee is None or merge_request.assignee.username not in candidates:
|
if len(review_candidates) > 0:
|
||||||
if len(candidates) > 0:
|
new_reviewer = random.choice(review_candidates)
|
||||||
new_reviewer = random.choice(candidates)
|
|
||||||
log.debug("%s is the new reviewer", new_reviewer)
|
log.debug("%s is the new reviewer", new_reviewer)
|
||||||
|
|
||||||
# find the user object for the new reviewer
|
# get the user object for the new reviewer
|
||||||
new_reviewer_obj = self.client.users.get_by_username(new_reviewer)
|
new_reviewer_obj = self.client.users.get_by_username(new_reviewer)
|
||||||
|
|
||||||
# create note for the update
|
# create note for the update
|
||||||
assign_msg = {'body': self.random_new_reviewer_message().format(approve_count,
|
msg = {'body': self.random_new_reviewer_message().format(new_reviewer)}
|
||||||
project.code_reviews_necessary,
|
self.client.project_mergerequest_notes.create(msg, project_id=project_obj.id,
|
||||||
new_reviewer)}
|
|
||||||
self.client.project_mergerequest_notes.create(assign_msg, project_id=project_obj.id,
|
|
||||||
merge_request_id=merge_request.id)
|
merge_request_id=merge_request.id)
|
||||||
|
|
||||||
# assign the merge request to the new reviewer
|
# assign the merge request to the new reviewer
|
||||||
|
|
@ -129,38 +187,31 @@ class GitlabBot(object):
|
||||||
else:
|
else:
|
||||||
log.warning("no reviewers left to review %s, doing nothing", merge_request.title)
|
log.warning("no reviewers left to review %s, doing nothing", merge_request.title)
|
||||||
else:
|
else:
|
||||||
log.info("%s is already assigned to a candidate reviewer", merge_request.title)
|
log.debug("needs_reviewer set but the request is assigned to a reviewer, doing nothing")
|
||||||
|
|
||||||
# regardless of our decision to assign or not, if we're supposed to send the reset
|
# if there's an accepter necessary, assign the merge request
|
||||||
# message, we should
|
if len(request_state.approver_list) >= project.code_reviews_necessary and request_state.needs_accepter:
|
||||||
if send_reset:
|
log.debug("%s needs an accepter", merge_request.title)
|
||||||
log.info("adding reset message to %s", merge_request.title)
|
|
||||||
reset_msg = {'body': self.random_counter_reset_message().format(project.code_reviews_necessary)}
|
|
||||||
self.client.project_mergerequest_notes.create(reset_msg, project_id=project_obj.id,
|
|
||||||
merge_request_id=merge_request.id)
|
|
||||||
else:
|
|
||||||
log.debug("%s has enough code reviews, it needs someone to merge it", merge_request.title)
|
|
||||||
if merge_request.assignee is not None:
|
if merge_request.assignee is not None:
|
||||||
log.debug("%s currently assigned to %s", merge_request.title, merge_request.assignee.username)
|
log.debug("%s currently assigned to %s", merge_request.title, merge_request.assignee.username)
|
||||||
excluded = [merge_request.author.username]
|
|
||||||
candidates = [x for x in project.code_review_final_merge_assignees.split(',') if x not in excluded]
|
|
||||||
if merge_request.assignee is None or merge_request.assignee.username not in candidates:
|
|
||||||
if len(candidates) > 0:
|
|
||||||
new_approver = random.choice(candidates)
|
|
||||||
log.debug("%s is the new approver", new_approver)
|
|
||||||
|
|
||||||
# find the user object for the new reviewer
|
if merge_request.assignee is None or merge_request.assignee.username not in accept_candidates:
|
||||||
new_approver_obj = self.client.users.get_by_username(new_approver)
|
if len(accept_candidates) > 0:
|
||||||
|
new_accepter = random.choice(accept_candidates)
|
||||||
|
log.debug("%s is the new accepter", new_accepter)
|
||||||
|
|
||||||
|
# get the user object for the new accepter
|
||||||
|
new_accepter_obj = self.client.users.get_by_username(new_accepter)
|
||||||
|
|
||||||
# create note for the update
|
# create note for the update
|
||||||
assign_msg = {'body': self.random_reviews_done_message().format(new_approver)}
|
msg = {'body': self.random_reviews_done_message().format(new_accepter)}
|
||||||
self.client.project_mergerequest_notes.create(assign_msg, project_id=project_obj.id,
|
self.client.project_mergerequest_notes.create(msg, project_id=project_obj.id,
|
||||||
merge_request_id=merge_request.id)
|
merge_request_id=merge_request.id)
|
||||||
|
|
||||||
# assign the merge request to the new reviewer
|
# assign the merge request to the new reviewer
|
||||||
self.client.update(merge_request, assignee_id=new_approver_obj.id)
|
self.client.update(merge_request, assignee_id=new_accepter_obj.id)
|
||||||
else:
|
else:
|
||||||
log.warning("no approvers left to approve %s, doing nothing", merge_request.title)
|
log.warning("no accepters left to accept %s, doing nothing", merge_request.title)
|
||||||
|
|
||||||
|
|
||||||
class _MergeRequestScanningState(object):
|
class _MergeRequestScanningState(object):
|
||||||
|
|
@ -171,7 +222,9 @@ class _MergeRequestScanningState(object):
|
||||||
"""Set default flags/values."""
|
"""Set default flags/values."""
|
||||||
self.approver_list = []
|
self.approver_list = []
|
||||||
|
|
||||||
|
self.unlogged_review_start = True
|
||||||
self.unlogged_approval_reset = False
|
self.unlogged_approval_reset = False
|
||||||
self.unlogged_approval = False
|
self.unlogged_approval = False
|
||||||
self.needs_reviewer = False
|
self.unlogged_review_complete = False
|
||||||
|
self.needs_reviewer = True
|
||||||
self.needs_accepter = False
|
self.needs_accepter = False
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue