Merge pull request #684 from jberry-suse/leaper-comment-to-reviewbot

ReviewBot: refactor leaper comment from log functionality.

ReviewBot.py

@@ -25,6 +25,7 @@
 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 @@ def check_action_<type>(self, req, action):
     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'<!-- (?P<bot>[^ ]+) state=(?P<state>[^ ]+)(?: result=(?P<result>[^ ]+))? -->')
+
     # map of default config entries
     config_defaults = {
             # list of tuples (prefix, apiurl, submitrequestprefix)
@@ -73,6 +76,7 @@ def __init__(self, apiurl = None, dryrun = False, logger = None, user = None, gr
         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 @@ def set_request_ids_project(self, project, typename):
             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 = '<!-- {} state={} result={} -->'.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)

leaper.py

@@ -41,29 +41,12 @@
 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 @@ def __init__(self, *args, **kwargs):
         self.needs_check_source = False
         self.check_source_group = None
 
-        self.comment_marker_re = re.compile(r'<!-- leaper state=(?P<state>done|seen)(?: result=(?P<result>accepted|declined))? -->')
-
-        self.comment_log = None
-        self.commentlogger = LogToString(self, 'comment_log')
-        self.logger.addFilter(self.commentlogger)
-
         # project => package list
         self.packages = {}
 
@@ -374,7 +351,7 @@ def check_one_request(self, req):
         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 @@ def check_one_request(self, req):
         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 @@ def check_one_request(self, req):
             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 @@ def check_action__default(self, req, a):
         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 = "<!-- leaper state=%s%s -->\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