From 015d59cdc749b82138b35464828ce28172453490 Mon Sep 17 00:00:00 2001 From: Jimmy Berry Date: Fri, 10 Feb 2017 15:41:45 -0600 Subject: [PATCH] ReviewBot: refactor leaper comment from log functionality. Simple interface: - comment_handler_add(): Add handler to start recording log messages for comment. - comment_write(): Write comment from log messages if not similar to previous comment. See leaper.py for example usage. --- ReviewBot.py | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ leaper.py | 64 +++------------------------------------------------- 2 files changed, 64 insertions(+), 61 deletions(-) diff --git a/ReviewBot.py b/ReviewBot.py index 1a89e768..6efcf90e 100644 --- a/ReviewBot.py +++ b/ReviewBot.py @@ -25,6 +25,7 @@ import logging from optparse import OptionParser import cmdln from collections import namedtuple +from osclib.comments import CommentAPI from osclib.memoize import memoize import signal import datetime @@ -51,6 +52,8 @@ class ReviewBot(object): DEFAULT_REVIEW_MESSAGES = { 'accepted' : 'ok', 'declined': 'review failed' } REVIEW_CHOICES = ('normal', 'no', 'accept', 'accept-onpass', 'fallback-onfail', 'fallback-always') + COMMENT_MARKER_REGEX = re.compile(r'') + # map of default config entries config_defaults = { # list of tuples (prefix, apiurl, submitrequestprefix) @@ -73,6 +76,7 @@ class ReviewBot(object): self._review_mode = 'normal' self.fallback_user = None self.fallback_group = None + self.comment_api = CommentAPI(self.apiurl) self.load_config() @@ -375,6 +379,63 @@ class ReviewBot(object): req.read(request) self.requests.append(req) + def comment_handler_add(self, level=logging.INFO): + """Add handler to start recording log messages for comment.""" + self.comment_handler = CommentFromLogHandler(level) + self.logger.addHandler(self.comment_handler) + + def comment_handler_remove(self): + self.logger.removeHandler(self.comment_handler) + + def comment_find(self, request=None, state=None, result=None): + """Return previous comments by current bot and matching criteria.""" + # Case-insensitive for backwards compatibility. + bot = self.__class__.__name__.lower() + comments = self.comment_api.get_comments(request_id=request.reqid) + for c in comments.values(): + m = ReviewBot.COMMENT_MARKER_REGEX.match(c['comment']) + if m and \ + bot == m.group('bot').lower() and \ + (state is None or state == m.group('state')) and \ + (result is None or result == m.group('result')): + return c['id'], m.group('state'), m.group('result'), c['comment'] + return None, None, None, None + + def comment_write(self, state='done', result=None, request=None, message=None): + """Write comment from log messages if not similar to previous comment.""" + if request is None: + request = self.request + if message is None: + message = '\n\n'.join(self.comment_handler.lines) + + marker = ''.format(self.__class__.__name__, state, result) + message = marker + '\n\n' + message + + comment_id, _, _, comment_text = self.comment_find(request, state, result) + if comment_id is not None and comment_text.count('\n') == message.count('\n'): + # Assume same state/result and number of lines in message is duplicate. + self.logger.debug('previous comment too similar to bother commenting again') + return + + self.logger.debug('adding comment to {}: {}'.format(request.reqid, message)) + + if not self.dryrun: + if comment_id is not None: + self.comment_api.delete(comment_id) + self.comment_api.add_comment(request_id=request.reqid, comment=str(message)) + + self.comment_handler_remove() + + +class CommentFromLogHandler(logging.Handler): + def __init__(self, level=logging.INFO): + super(CommentFromLogHandler, self).__init__(level) + self.lines = [] + + def emit(self, record): + self.lines.append(record.getMessage()) + + class CommandLineInterface(cmdln.Cmdln): def __init__(self, *args, **kwargs): cmdln.Cmdln.__init__(self, args, kwargs) diff --git a/leaper.py b/leaper.py index 5d0e3482..e9c90a5b 100755 --- a/leaper.py +++ b/leaper.py @@ -41,29 +41,12 @@ import ReviewBot from check_maintenance_incidents import MaintenanceChecker from check_source_in_factory import FactorySourceChecker -from osclib.comments import CommentAPI - -class LogToString(logging.Filter): - def __init__(self, obj, propname): - self.obj = obj - self.propname = propname - - def filter(self, record): - if record.levelno >= logging.INFO: - line = record.getMessage() - comment_log = getattr(self.obj, self.propname) - if comment_log is not None: - comment_log.append(line) - setattr(self.obj, self.propname, comment_log) - return True - class Leaper(ReviewBot.ReviewBot): def __init__(self, *args, **kwargs): ReviewBot.ReviewBot.__init__(self, *args, **kwargs) self.do_comments = True - self.commentapi = CommentAPI(self.apiurl) self.maintbot = MaintenanceChecker(*args, **kwargs) # for FactorySourceChecker @@ -79,12 +62,6 @@ class Leaper(ReviewBot.ReviewBot): self.needs_check_source = False self.check_source_group = None - self.comment_marker_re = re.compile(r'') - - self.comment_log = None - self.commentlogger = LogToString(self, 'comment_log') - self.logger.addFilter(self.commentlogger) - # project => package list self.packages = {} @@ -374,7 +351,7 @@ class Leaper(ReviewBot.ReviewBot): self.needs_release_manager = False self.pending_factory_submission = False self.source_in_factory = None - self.comment_log = [] + self.comment_handler_add() self.packages = {} if len(req.actions) != 1: @@ -400,7 +377,7 @@ class Leaper(ReviewBot.ReviewBot): elif self.needs_release_manager: self.logger.info("request needs review by release management") - if self.comment_log: + if self.do_comments: result = None if request_ok is None: state = 'seen' @@ -410,8 +387,7 @@ class Leaper(ReviewBot.ReviewBot): else: state = 'done' result = 'declined' - self.add_comment(req, '\n\n'.join(self.comment_log), state) - self.comment_log = None + self.comment_write(state, result) if self.needs_release_manager: add_review = True @@ -458,40 +434,6 @@ class Leaper(ReviewBot.ReviewBot): self.logger.debug("auto decline request type %s"%a.type) return False - # TODO: make generic, move to Reviewbot. Used by multiple bots - def add_comment(self, req, msg, state, result=None): - if not self.do_comments: - return - - comment = "\n" % (state, ' result=%s' % result if result else '') - comment += "\n" + msg - - (comment_id, comment_state, comment_result, comment_text) = self.find_obs_request_comment(req, state) - - if comment_id is not None and state == comment_state: - # count number of lines as aproximation to avoid spamming requests - # for slight wording changes in the code - if len(comment_text.split('\n')) == len(comment.split('\n')): - self.logger.debug("not worth the update, previous comment %s is state %s", comment_id, comment_state) - return - - self.logger.debug("adding comment to %s, state %s result %s", req.reqid, state, result) - self.logger.debug("message: %s", msg) - if not self.dryrun: - if comment_id is not None: - self.commentapi.delete(comment_id) - self.commentapi.add_comment(request_id=req.reqid, comment=str(comment)) - - def find_obs_request_comment(self, req, state=None): - """Return previous comments (should be one).""" - if self.do_comments: - comments = self.commentapi.get_comments(request_id=req.reqid) - for c in comments.values(): - m = self.comment_marker_re.match(c['comment']) - if m and (state is None or state == m.group('state')): - return c['id'], m.group('state'), m.group('result'), c['comment'] - return None, None, None, None - def check_action__default(self, req, a): self.logger.info("unhandled request type %s"%a.type) self.needs_release_manager = True