launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/remove-old-status into lp:launchpad-buildd.

Commit message:
Make the status XML-RPC method a synonym for status_dict.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/remove-old-status/+merge/191809

Now that buildd-manager is using status_dict, we can take the next step of the plan I outlined in https://code.launchpad.net/~cjwatson/launchpad-buildd/better-status/+merge/189675, and make status a synonym for status_dict.
-- 
https://code.launchpad.net/~cjwatson/launchpad-buildd/remove-old-status/+merge/191809
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/remove-old-status into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2013-10-18 11:24:41 +0000
+++ debian/changelog	2013-10-18 13:25:03 +0000
@@ -3,6 +3,7 @@
   * Unset LANG and LC_ALL rather than setting them to C, and unset a number
     of other environment variables too (including DISPLAY and TERM), in line
     with Debian buildds.
+  * Make the status XML-RPC method a synonym for status_dict.
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Fri, 18 Oct 2013 12:24:00 +0100
 

=== modified file 'lpbuildd/slave.py'
--- lpbuildd/slave.py	2013-10-07 21:19:05 +0000
+++ lpbuildd/slave.py	2013-10-18 13:25:03 +0000
@@ -655,22 +655,6 @@
                 self._builders.keys())
 
     def xmlrpc_status(self):
-        """Return the status of the build daemon.
-
-        Depending on the builder status we return differing amounts of
-        data. We do however always return the builder status as the first
-        value.
-
-        This method is deprecated in favour of `xmlrpc_status_dict`.
-        """
-        status = self.slave.builderstatus
-        statusname = status.split('.')[-1]
-        func = getattr(self, "status_" + statusname, None)
-        if func is None:
-            raise ValueError("Unknown status '%s'" % status)
-        return (status, ) + func()
-
-    def xmlrpc_status_dict(self):
         """Return the status of the build daemon, as a dictionary.
 
         Depending on the builder status we return differing amounts of data,
@@ -678,7 +662,7 @@
         """
         status = self.slave.builderstatus
         statusname = status.split('.')[-1]
-        func = getattr(self, "status_dict_" + statusname, None)
+        func = getattr(self, "status_" + statusname, None)
         if func is None:
             raise ValueError("Unknown status '%s'" % status)
         ret = {"builder_status": status}
@@ -687,17 +671,11 @@
         ret.update(func())
         return ret
 
+    def xmlrpc_status_dict(self):
+        return self.xmlrpc_status()
+
     def status_IDLE(self):
-        """Handler for xmlrpc_status IDLE.
-
-        Returns a tuple containing a empty string since there's nothing
-        to report.
-        """
-        # keep the result code sane
-        return ('', )
-
-    def status_dict_IDLE(self):
-        """Handler for xmlrpc_status_dict IDLE."""
+        """Handler for xmlrpc_status IDLE."""
         return {}
 
     def status_BUILDING(self):
@@ -706,14 +684,6 @@
         Returns the build id and up to one kilobyte of log tail.
         """
         tail = self.slave.getLogTail()
-        return (self.buildid, xmlrpclib.Binary(tail))
-
-    def status_dict_BUILDING(self):
-        """Handler for xmlrpc_status_dict BUILDING.
-
-        Returns the build id and up to one kilobyte of log tail.
-        """
-        tail = self.slave.getLogTail()
         return {"build_id": self.buildid, "logtail": xmlrpclib.Binary(tail)}
 
     def status_WAITING(self):
@@ -723,19 +693,6 @@
         unless the builder failed in which case we return the buildstatus
         and the build id but no file set.
         """
-        if self.slave.buildstatus in (BuildStatus.OK, BuildStatus.PACKAGEFAIL,
-                                      BuildStatus.DEPFAIL):
-            return (self.slave.buildstatus, self.buildid,
-                    self.slave.waitingfiles, self.slave.builddependencies)
-        return (self.slave.buildstatus, self.buildid)
-
-    def status_dict_WAITING(self):
-        """Handler for xmlrpc_status_dict WAITING.
-
-        Returns the build id and the set of files waiting to be returned
-        unless the builder failed in which case we return the buildstatus
-        and the build id but no file set.
-        """
         ret = {
             "build_status": self.slave.buildstatus,
             "build_id": self.buildid,
@@ -753,15 +710,6 @@
         not able to do anything else than answer its status, so returns the
         build id only.
         """
-        return (self.buildid, )
-
-    def status_dict_ABORTING(self):
-        """Handler for xmlrpc_status_dict ABORTING.
-
-        This state means the builder is performing the ABORT command and is
-        not able to do anything else than answer its status, so returns the
-        build id only.
-        """
         return {"build_id": self.buildid}
 
     def xmlrpc_ensurepresent(self, sha1sum, url, username, password):

=== modified file 'lpbuildd/tests/harness.py'
--- lpbuildd/tests/harness.py	2013-10-07 21:19:05 +0000
+++ lpbuildd/tests/harness.py	2013-10-18 13:25:03 +0000
@@ -92,8 +92,8 @@
     sourcepackagerecipe
     translation-templates
 
-    >>> s.status()
-    ['BuilderStatus.IDLE', '']
+    >>> s.status()["builder_status"]
+    'BuilderStatus.IDLE'
 
     >>> s.status_dict()["builder_status"]
     'BuilderStatus.IDLE'