patchwork cleanups and enhacements

README.md

@@ -305,9 +305,8 @@ commits up-to-date, to allow newer patches to apply.
 ### Testing first patches
 
 Once your `baseline` command has finished, you will be able to start checking
-patches from a Patchwork instance. However, you would need to specify the
-patch to start from on the first run, and would need to use somewhat
-different commands for Patchwork v1 and v2 instances.
+patches from a Patchwork instance. Here are very similar examples of commands
+to be used with the Patchwork 1 and Patchwork 2 instances:
 
 For Patchwork v1 run:
 
@@ -322,22 +321,18 @@ and for Patchwork v2 run:
     sktm -v --jjname <JENKINS_PROJECT> patchwork \
          <GIT_REPO_URL> \
          --restapi <PATCHWORK_BASE_URL> <PATCHWORK_PROJECT> \
-         --lastpatch <PATCHWORK_TIMESTAMP> \
+         --lastpatch <PATCHWORK_PATCH_ID> \
          --skip <PATTERN> [... <PATTERN>]
 
 Here, `<PATCHWORK_BASE_URL>` would be the base URL of the Patchwork instance,
 `<PATCHWORK_PROJECT>` - the name of the Patchwork project to check for new
 patches. The `<PATCHWORK_PATCH_ID>` would be the ID of the newest patch (as
-seen in Patchwork URLs) to ignore. All patches with greater IDs will be
-considered for testing. `<PATCHWORK_TIMESTAMP>` would be a Patchwork instance
-timestamp of the newest patch to ignore, in a format acceptable by Python's
-`dateutil.parser` module, e.g. from the `Date:` header of a patch message. All
-patches with greater timestamps will be considered for testing. Finally, a list
-of regex `<PATTERN>`s (case insensitive) can be provided to skip testing of
-patches which match the patterns. This last option is useful for example if the
-Patchwork project contains patches for additional tools besides kernel ones.
-By default, git pull requests and tools from netdev list (`iproute`, `ethtool`
-etc.) are skipped.
+seen in Patchwork URLs) to ignore. All newer patches (with greater IDs) will be
+considered for testing. Finally, a list of regex `<PATTERN>`s (case
+insensitive) can be provided to skip testing of patches which match the
+patterns. This last option is useful for example if the Patchwork project
+contains patches for additional tools besides kernel ones. By default, git pull
+requests and tools from netdev list (`iproute`, `ethtool` etc.) are skipped.
 
 E.g. this command would test all patches after the one with ID 10363835, from
 the "scsi" tree's Patchwork v1 instance, applying them onto the latest
@@ -354,7 +349,7 @@ Patchwork v2 instance after `Thu,  3 May 2018 14:35:00 +0100` timestamp:
     sktm -v --jjname sktm patchwork \
          git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git \
          --restapi https://patchwork.ozlabs.org netdev \
-         --lastpatch 'Thu,  3 May 2018 14:35:00 +0100'
+         --lastpatch 823457
 
 Note: do not run the commands above with the `--lastpatch` option value
 intact, as that would likely result in a lot of Jenkins jobs submitted,

sktm/__init__.py

@@ -92,9 +92,7 @@ def add_pw(self, baseurl, pname, lpatch=None, restapi=False, apikey=None,
         Args:
             baseurl:        Patchwork base URL.
             pname:          Patchwork project name.
-            lpatch:         Last processed patch. Patch ID, if adding an XML
-                            RPC-based interface. Patch timestamp, if adding a
-                            REST-based interface. Can be omitted to
+            lpatch:         ID of the last processed patch. Can be omitted to
                             retrieve one from the database.
             restapi:        True if the REST API to Patchwork should be used.
                             False implies XMLRPC interface.
@@ -107,7 +105,6 @@ def add_pw(self, baseurl, pname, lpatch=None, restapi=False, apikey=None,
                 baseurl, pname, lpatch, apikey, skip
             )
 
-            # FIXME Figure out the last patch first, then create the interface
             if lpatch is None:
                 lcdate = self.db.get_last_checked_patch_date(baseurl,
                                                              pw.project_id)
@@ -121,10 +118,9 @@ def add_pw(self, baseurl, pname, lpatch=None, restapi=False, apikey=None,
                 pw.since = since
         else:
             pw = sktm.patchwork.PatchworkV1Project(
-                baseurl, pname, int(lpatch) if lpatch else None, skip
+                baseurl, pname, lpatch, skip
             )
 
-            # FIXME Figure out the last patch first, then create the interface
             if lpatch is None:
                 lcpatch = self.db.get_last_checked_patch(baseurl,
                                                          pw.project_id)
@@ -210,9 +206,6 @@ def get_patch_info_from_url(self, interface, patch_url):
         baseurl = match.group(1)
         patch_id = int(match.group(2))
         patch = interface.get_patch_by_id(patch_id)
-        if patch is None:
-            raise Exception('Can\'t get data for %s' % patch_url)
-
         logging.info('patch: [%d] %s', patch_id, patch.get('name'))
 
         if isinstance(interface, sktm.patchwork.PatchworkV2Project):

sktm/executable.py

@@ -106,8 +106,8 @@ def setup_parser():
     parser_patchwork.add_argument("repo", type=str, help="Base repo URL")
     parser_patchwork.add_argument("baseurl", type=str, help="Base URL")
     parser_patchwork.add_argument("project", type=str, help="Project name")
-    parser_patchwork.add_argument("--lastpatch", type=str, help="Last patch "
-                                  "(id for pw1; datetime for pw2)")
+    parser_patchwork.add_argument("--lastpatch", type=int,
+                                  help="Last patch ID")
     parser_patchwork.add_argument("--restapi", help="Use REST API",
                                   action="store_true", default=False)
     parser_patchwork.add_argument("--apikey", type=str,

sktm/patchwork.py

@@ -418,33 +418,46 @@ def _get_mbox_url_sfx(self):
 
         return "mbox"
 
+    def _get_project_id(self, project_name):
+        """
+        Get project ID based on its name. Child classes need to implement
+        this method.
+
+        Args:
+            project_name:  The name of the project to retrieve.
+
+        Returns:
+            Integer representing project's ID.
+        """
+        raise NotImplementedError
+
 
 class PatchworkV2Project(PatchworkProject):
     """
     A Patchwork REST interface
     """
-    def __init__(self, baseurl, projectname, since, apikey=None, skip=[]):
+    def __init__(self, baseurl, projectname, lastpatch, apikey=None, skip=[]):
         """
         Initialize a Patchwork REST interface.
 
         Args:
             baseurl:        Patchwork base URL.
             projectname:    Patchwork project name, or None.
-            since:          Last processed patch timestamp in a format
-                            accepted by dateutil.parser.parse. Patches with
-                            this or earlier timestamp will be ignored.
+            lastpatch:      ID of the last processed patch. Only patches newer
+                            than this one will be processed.
             apikey:         Patchwork API authentication token.
             skip:           List of additional regex patterns to skip in patch
                             names, case insensitive.
         """
-        # Last processed patch timestamp in a dateutil.parser.parse format
-        self.since = since
-        # TODO Describe
-        self.nsince = None
         # Patchwork API authentication token.
         self.apikey = apikey
         # JSON representation of API URLs retrieved from the Patchwork server
         self.apiurls = self.__get_apiurls(baseurl)
+        # Get the datetime of the passed lastpatch to use in patch filtering
+        if lastpatch:
+            self.since = self.get_patch_by_id(lastpatch).get('date')
+        else:
+            self.since = None
         super(PatchworkV2Project, self).__init__(baseurl, projectname, skip)
 
     def _get_project_id(self, project_name):
@@ -749,16 +762,15 @@ def get_patchsets(self, patchlist):
         # For each patch ID
         for pid in patchlist:
             patch = self.get_patch_by_id(pid)
-            if patch:
-                # For each series the patch belongs to
-                for series in patch.get("series"):
-                    sid = series.get("id")
-                    if sid not in seen:
-                        series_list += self.__get_series_from_url(
-                            join_with_slash(self.apiurls.get("series"),
-                                            str(sid))
-                        )
-                        seen.add(sid)
+            # For each series the patch belongs to
+            for series in patch.get("series"):
+                sid = series.get("id")
+                if sid not in seen:
+                    series_list += self.__get_series_from_url(
+                        join_with_slash(self.apiurls.get("series"),
+                                        str(sid))
+                    )
+                    seen.add(sid)
 
         return series_list
 
@@ -819,7 +831,7 @@ def __get_rpc(self, baseurl):
             ver = rpc.pw_rpc_version()
             # check for normal patchwork1 xmlrpc version numbers
             if not (ver == [1, 3, 0] or ver == 1):
-                raise Exception("Unknown xmlrpc version %s", ver)
+                raise Exception("Unknown xmlrpc version %s" % ver)
 
         except xmlrpclib.Fault as err:
             if err.faultCode == 1 and \
@@ -828,14 +840,14 @@ def __get_rpc(self, baseurl):
                 rpc = RpcWrapper(rpc)
                 ver = rpc.pw_rpc_version()
                 if ver < 1010:
-                    raise Exception("Unsupported xmlrpc version %s", ver)
+                    raise Exception("Unsupported xmlrpc version %s" % ver)
 
                 # grab extra info for later parsing
                 self.fields = ['id', 'name', 'submitter', 'msgid',
                                ['root_comment', ['headers']],
                                'date', 'project_id']
             else:
-                raise Exception("Unknown xmlrpc fault: %s", err.faultString)
+                raise Exception("Unknown xmlrpc fault: %s" % err.faultString)
 
         return rpc
 
@@ -895,8 +907,7 @@ def get_patch_by_id(self, pid):
             patch = self.rpc.patch_get(pid, self.fields)
 
         if patch is None or patch == {}:
-            logging.warning("Failed to get data for patch %d", pid)
-            patch = None
+            raise Exception('Can\'t get patch by id %d)'.format(pid))
 
         self.__update_patch_name(patch)
 
@@ -942,28 +953,28 @@ def set_patch_check(self, pid, jurl, result):
         pass
 
     # TODO Move this to __init__ or make it a class method
-    def _get_project_id(self, projectname):
+    def _get_project_id(self, project_name):
         """
         Retrieve ID of the project with the specified name.
 
         Args:
-            projectname:    The name of the project to retrieve ID for.
+            project_name:    The name of the project to retrieve ID for.
 
         Returns:
-            The project name.
+            Integer representing project's ID.
 
         Raises:
             A string containing an error message, if the project with the
             specified name was not found.
         """
-        plist = self.rpc.project_list(projectname)
+        plist = self.rpc.project_list(project_name)
         for project in plist:
-            if project.get("linkname") == projectname:
+            if project.get("linkname") == project_name:
                 pid = int(project.get("id"))
-                logging.debug("%s -> %d", projectname, pid)
+                logging.debug("%s -> %d", project_name, pid)
                 return pid
 
-        raise Exception("Couldn't find project %s" % projectname)
+        raise Exception("Couldn't find project %s" % project_name)
 
     # FIXME This doesn't just parse a patch. Name/refactor accordingly.
     def __parse_patch(self, patch):
@@ -1156,8 +1167,7 @@ def get_patchsets(self, patchlist):
         logging.debug("get_patchsets: %s", patchlist)
         for pid in patchlist:
             patch = self.get_patch_by_id(pid)
-            if patch:
-                pset = self.__parse_patch(patch)
-                if pset:
-                    series_list.append(pset)
+            pset = self.__parse_patch(patch)
+            if pset:
+                series_list.append(pset)
         return series_list