From 701021070c2aa8385f9d7a0758875ec4cb2522ea Mon Sep 17 00:00:00 2001 From: Jimmy Berry Date: Fri, 1 Nov 2019 13:51:23 -0500 Subject: [PATCH] ReviewBot: rework override check to operate on actions. In multi-action workflows the override check should be performed by action so that different groups of users can override for different actions. Additionally, abstracting the request_commands() method provides a flexible base for additional commands to be added by ReviewBots. --- ReviewBot.py | 46 +++++++++++++++++++++++++++------------------- origin-manager.py | 2 +- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/ReviewBot.py b/ReviewBot.py index 724e2246..444bedc0 100644 --- a/ReviewBot.py +++ b/ReviewBot.py @@ -182,20 +182,16 @@ class ReviewBot(object): with sentry_sdk.configure_scope() as scope: scope.set_extra('request.id', self.request.reqid) - override = self.request_override_check(req) - if override is not None: - good = override - else: - try: - good = self.check_one_request(req) - except Exception as e: - good = None + try: + good = self.check_one_request(req) + except Exception as e: + good = None - import traceback - traceback.print_exc() - return_value = 1 + import traceback + traceback.print_exc() + return_value = 1 - sentry_sdk.capture_exception(e) + sentry_sdk.capture_exception(e) if self.review_mode == 'no': good = None @@ -228,15 +224,12 @@ class ReviewBot(object): return users - def request_override_check(self, request, force=False): + def request_override_check(self, force=False): """Check for a comment command requesting review override.""" if not force and not self.override_allow: return None - comments = self.comment_api.get_comments(request_id=request.reqid) - users = self.request_override_check_users(request.actions[0].tgt_project) - for args, who in self.comment_api.command_find( - comments, self.review_user, 'override', users): + for args, who in self.request_commands('override'): message = 'overridden by {}'.format(who) override = args[1] if len(args) >= 2 else 'accept' if override == 'accept': @@ -247,6 +240,17 @@ class ReviewBot(object): self.review_messages['declined'] = message return False + def request_commands(self, command, who_allowed=None, request=None, action=None): + if not request: + request = self.request + if not action: + action = self.action + if not who_allowed: + who_allowed = self.request_override_check_users(action.tgt_project) + + comments = self.comment_api.get_comments(request_id=request.reqid) + yield from self.comment_api.command_find(comments, self.review_user, command, who_allowed) + def _set_review(self, req, state): doit = self.can_accept_review(req.reqid) if doit is None: @@ -407,8 +411,12 @@ class ReviewBot(object): with sentry_sdk.configure_scope() as scope: scope.set_extra('action.key', key) - func = getattr(self, self.action_method(a)) - ret = func(req, a) + override = self.request_override_check() + if override is not None: + ret = override + else: + func = getattr(self, self.action_method(a)) + ret = func(req, a) # In the case of multiple actions take the "lowest" result where the # order from lowest to highest is: False, None, True. diff --git a/origin-manager.py b/origin-manager.py index 520470c7..951e385b 100755 --- a/origin-manager.py +++ b/origin-manager.py @@ -86,7 +86,7 @@ class OriginManager(ReviewBot.ReviewBot): if result.wait: # Allow overriding a policy wait by accepting as workaround with the # hope that pending request will be accepted. - override = self.request_override_check(self.request, True) + override = self.request_override_check(True) if override: self.review_messages['accepted'] = origin_annotation_dump( origin_info_new, origin_info_old, self.review_messages['accepted'], raw=True)