kou commented on code in PR #34161: URL: https://github.com/apache/arrow/pull/34161#discussion_r1115091984
########## dev/archery/archery/bot.py: ########## @@ -82,6 +87,121 @@ def parse_args(self, ctx, args): group = partial(click.group, cls=Group) +LABEL_PREFIX = "awaiting" + + [email protected] +class PullRequestState(enum.Enum): + """State of a pull request.""" + + review = f"{LABEL_PREFIX} review" + committer_review = f"{LABEL_PREFIX} committer review" + changes = f"{LABEL_PREFIX} changes" + change_review = f"{LABEL_PREFIX} change review" + merge = f"{LABEL_PREFIX} merge" + + +COMMITTER_ROLES = {'OWNER', 'MEMBER'} + + +class PullRequestWorkflowBot: + + def __init__(self, event_name, event_payload, token=None): + self.github = github.Github(token) + self.event_name = event_name + self.event_payload = event_payload + + @cached_property + def pull(self): + """ + Returns a github.PullRequest object associated with the event. + """ + return self.repo.get_pull(self.event_payload['pull_request']['number']) + + @cached_property + def repo(self): + return self.github.get_repo(self.event_payload['repository']['id'], lazy=True) + + def handle(self): + current_state = None + try: + current_state = self.get_current_state() + except EventError: + # In case of error (more than one state) we clear state labels + # only possible if a label has been manually added. + self.clear_current_state() + new_state = self.get_target_state(current_state) + if current_state != new_state.value: + if current_state: + self.clear_current_state() + self.set_state(new_state) + + def get_current_state(self): + """ + Returns a string with the current PR state label + based on label starting with LABEL_PREFIX. + If more than one label is found raises EventError. + If no label is found returns None. + """ + states = [label.name for label in self.pull.get_labels() + if label.name.startswith(LABEL_PREFIX)] + if len(states) > 1: + raise EventError(f"PR cannot be on more than one states - {states}") + elif states: + return states[0] + + def clear_current_state(self): + """ + Removes all existing labels starting with LABEL_PREFIX + """ + for label in self.pull.get_labels(): + if label.name.startswith(LABEL_PREFIX): + self.pull.remove_from_labels(label) + + def get_target_state(self, current_state): Review Comment: How about using more meaningful method name such as `transit_state` and `compute_next_state`? ########## dev/archery/archery/bot.py: ########## @@ -82,6 +87,121 @@ def parse_args(self, ctx, args): group = partial(click.group, cls=Group) +LABEL_PREFIX = "awaiting" + + [email protected] +class PullRequestState(enum.Enum): + """State of a pull request.""" + + review = f"{LABEL_PREFIX} review" + committer_review = f"{LABEL_PREFIX} committer review" + changes = f"{LABEL_PREFIX} changes" + change_review = f"{LABEL_PREFIX} change review" + merge = f"{LABEL_PREFIX} merge" + + +COMMITTER_ROLES = {'OWNER', 'MEMBER'} + + +class PullRequestWorkflowBot: + + def __init__(self, event_name, event_payload, token=None): + self.github = github.Github(token) + self.event_name = event_name + self.event_payload = event_payload + + @cached_property + def pull(self): + """ + Returns a github.PullRequest object associated with the event. + """ + return self.repo.get_pull(self.event_payload['pull_request']['number']) + + @cached_property + def repo(self): + return self.github.get_repo(self.event_payload['repository']['id'], lazy=True) + + def handle(self): + current_state = None + try: + current_state = self.get_current_state() + except EventError: + # In case of error (more than one state) we clear state labels + # only possible if a label has been manually added. + self.clear_current_state() + new_state = self.get_target_state(current_state) + if current_state != new_state.value: + if current_state: + self.clear_current_state() + self.set_state(new_state) + + def get_current_state(self): + """ + Returns a string with the current PR state label + based on label starting with LABEL_PREFIX. + If more than one label is found raises EventError. + If no label is found returns None. + """ + states = [label.name for label in self.pull.get_labels() + if label.name.startswith(LABEL_PREFIX)] + if len(states) > 1: + raise EventError(f"PR cannot be on more than one states - {states}") + elif states: + return states[0] + + def clear_current_state(self): + """ + Removes all existing labels starting with LABEL_PREFIX + """ + for label in self.pull.get_labels(): + if label.name.startswith(LABEL_PREFIX): + self.pull.remove_from_labels(label) + + def get_target_state(self, current_state): + """ + Returns the expected target state based on the event and + the current state. + """ + if (self.event_name == "pull_request_target" and + self.event_payload['action'] == 'opened'): + if (self.event_payload['pull_request']['author_association'] in + COMMITTER_ROLES): + return PullRequestState.committer_review + else: + return PullRequestState.review + elif (self.event_name == "pull_request_review" and + self.event_payload["action"] == "submitted"): + review_state = self.event_payload["review"]["state"].lower() + is_committer_review = (self.event_payload['review']['author_association'] + in COMMITTER_ROLES) + if not is_committer_review: + # Non-committer reviews cannot change state once committer has already + # reviewed and requested changes. + if current_state in ( + PullRequestState.change_review.value, + PullRequestState.changes.value): + return PullRequestState(current_state) + else: + return PullRequestState.committer_review + if review_state == 'approved': + return PullRequestState.merge + elif review_state in ("changes_requested", "commented"): Review Comment: Can we simplify this? ```suggestion else ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
