diff --git a/dr_botzo/gitlab_bot/lib.py b/dr_botzo/gitlab_bot/lib.py index 15fd9dd..95f7e93 100644 --- a/dr_botzo/gitlab_bot/lib.py +++ b/dr_botzo/gitlab_bot/lib.py @@ -15,13 +15,16 @@ class GitlabBot(object): """Bot for doing GitLab stuff. Might be used by the IRC bot, or daemons, or whatever.""" - COUNTER_RESET_FORMAT = "Review counter reset." - COUNTER_START_FORMAT = "Please obtain {0:d} reviews." - NEW_REVIEWER_FORMAT = "{0:d} of {1:d} reviewed. Assigning to {2:s} to review." - REVIEWS_DONE_FORMAT = "Reviews conducted. Assigning to {0:s} to approve." + REVIEWS_START_FORMAT = "Starting review process." + REVIEWS_RESET_FORMAT = "Review process reset." + REVIEWS_REVIEW_REGISTERED_FORMAT = "Review identified." + 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 = [ "The computer is your friend.", - "Thank you for complying.", ] def __init__(self): @@ -30,66 +33,26 @@ class GitlabBot(object): self.client = gitlab.Gitlab(config.url, private_token=config.token) self.client.auth() - def random_counter_reset_message(self): - return "{0:s} {1:s} {2:s}".format(self.COUNTER_RESET_FORMAT, self.COUNTER_START_FORMAT, + def random_reviews_start_message(self): + return "{0:s} {1:s} {2:s}".format(self.REVIEWS_START_FORMAT, self.REVIEWS_PROGRESS_FORMAT, 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): return "{0:s} {1:s}".format(self.NEW_REVIEWER_FORMAT, random.choice(self.NOTE_COMMENT)) def random_reviews_done_message(self): - return "{0:s} {1:s}".format(self.REVIEWS_DONE_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 + return "{0:s} {1:s}".format(self.NEW_ACCEPTER_FORMAT, random.choice(self.NOTE_COMMENT)) def scan_project_for_reviews(self, project): 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'): log.debug("scanning merge request '%s'", merge_request.title) - # check to see if the merge request needs a reviewer or a merge - approve_count, approvers, send_reset = self.get_code_review_acceptance_count(merge_request) - if approve_count < project.code_reviews_necessary: + # moved this method into here + # # check to see if the merge request needs a reviewer or a merge + # 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) if merge_request.assignee is not None: 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 candidates: - if len(candidates) > 0: - new_reviewer = random.choice(candidates) + if merge_request.assignee is None or merge_request.assignee.username not in review_candidates: + if len(review_candidates) > 0: + new_reviewer = random.choice(review_candidates) 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) # create note for the update - assign_msg = {'body': self.random_new_reviewer_message().format(approve_count, - project.code_reviews_necessary, - new_reviewer)} - self.client.project_mergerequest_notes.create(assign_msg, project_id=project_obj.id, + msg = {'body': self.random_new_reviewer_message().format(new_reviewer)} + self.client.project_mergerequest_notes.create(msg, project_id=project_obj.id, merge_request_id=merge_request.id) # assign the merge request to the new reviewer @@ -129,38 +187,31 @@ class GitlabBot(object): else: log.warning("no reviewers left to review %s, doing nothing", merge_request.title) 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 - # message, we should - if send_reset: - 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 there's an accepter necessary, assign the merge request + if len(request_state.approver_list) >= project.code_reviews_necessary and request_state.needs_accepter: + log.debug("%s needs an accepter", merge_request.title) if merge_request.assignee is not None: 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 - new_approver_obj = self.client.users.get_by_username(new_approver) + if merge_request.assignee is None or merge_request.assignee.username not in accept_candidates: + 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 - assign_msg = {'body': self.random_reviews_done_message().format(new_approver)} - self.client.project_mergerequest_notes.create(assign_msg, project_id=project_obj.id, + msg = {'body': self.random_reviews_done_message().format(new_accepter)} + self.client.project_mergerequest_notes.create(msg, project_id=project_obj.id, merge_request_id=merge_request.id) # 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: - 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): @@ -171,7 +222,9 @@ class _MergeRequestScanningState(object): """Set default flags/values.""" self.approver_list = [] + self.unlogged_review_start = True self.unlogged_approval_reset = False self.unlogged_approval = False - self.needs_reviewer = False + self.unlogged_review_complete = False + self.needs_reviewer = True self.needs_accepter = False