launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/lpbuildbot/poll-json into lp:lpbuildbot.

Commit message:
Convert buildbot-poll to buildbot's JSON status endpoint.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/lpbuildbot/poll-json/+merge/344971
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/lpbuildbot/poll-json into lp:lpbuildbot.

=== modified file 'buildbot-poll.py'
--- buildbot-poll.py	2018-05-02 09:55:50 +0000
+++ buildbot-poll.py	2018-05-02 16:56:23 +0000
@@ -1,5 +1,5 @@
 #!/usr/bin/python
-# Copyright 2008 Canonical Ltd.  All rights reserved.
+# Copyright 2008-2018 Canonical Ltd.  All rights reserved.
 
 """Query our buildbot and do stuff.
 
@@ -17,13 +17,11 @@
 import os.path
 import subprocess
 import sys
-from urllib2 import (
-    build_opener, HTTPCookieProcessor, HTTPError, HTTPRedirectHandler,
-    Request)
-from urlparse import urljoin, urlparse, urlunparse
+from urllib import quote
+from urlparse import urljoin
 import warnings
-import xmlrpclib
 
+from buildbot.status.results import Results
 import bzrlib.errors
 import bzrlib.bzrdir
 import bzrlib.missing
@@ -31,6 +29,7 @@
 from bzrlib import config
 from bzrlib.plugins.pqm import pqm_submit
 from bzrlib.smtp_connection import SMTPConnection
+import requests
 
 
 class GPGStrategy(gpg.GPGStrategy):
@@ -114,94 +113,29 @@
     }
 
 
-# XMLRPCRedirectHandler and UrlLib2Transport taken from Launchpad's
-# canonical.launchpad.components.externalbugtracker.xmlrpc module.
-# It is not imported from that location because this script needs
-# to be standalone.
-class XMLRPCRedirectHandler(HTTPRedirectHandler):
-    """A handler for HTTP redirections of XML-RPC requests."""
-
-    def redirect_request(self, req, fp, code, msg, headers, newurl):
-        """Return a Request or None in response to a redirect.
-
-        See `urllib2.HTTPRedirectHandler`.
-
-        If the original request is a POST request, the request's payload
-        will be preserved in the redirect and the returned request will
-        also be a POST request.
-        """
-        # If we can't handle this redirect,
-        # HTTPRedirectHandler.redirect_request() will raise an
-        # HTTPError. We call the superclass here in the old fashion
-        # since HTTPRedirectHandler isn't a new-style class.
-        new_request = HTTPRedirectHandler.redirect_request(
-            self, req, fp, code, msg, headers, newurl)
-
-        # If the old request is a POST request, the payload will be
-        # preserved. Note that we don't need to test for the POST-ness
-        # of the old request; if its data attribute - its payload - is
-        # not None it's a POST request, if it's None it's a GET request.
-        # We can therefore just copy the data from the old request to
-        # the new without worrying about breaking things.
-        new_request.data = req.data
-        return new_request
-
-
-class UrlLib2Transport(xmlrpclib.Transport):
-    """An XMLRPC transport which uses urllib2.
-
-    This XMLRPC transport uses the Python urllib2 module to make the
-    request, and connects via the HTTP proxy specified in the
-    environment variable `http_proxy`, i present. It also handles
-    cookies correctly, and in addition allows specifying the cookie
-    explicitly by setting `self.auth_cookie`.
-
-    Note: this transport isn't fit for general XML-RPC use. It is just
-    good enough for some of our extrnal bug tracker implementations.
-
-    :param endpoint: The URL of the XMLRPC server.
-    """
-
-    verbose = False
-
-    def __init__(self, endpoint, cookie_jar=None):
-        # Expected by Python 2.5+.
-        self._use_datetime = 0
-        self.scheme, self.host = urlparse(endpoint)[:2]
-        assert self.scheme in ('http', 'https'), (
-            "Unsupported URL schene: %s" % self.scheme)
-        self.cookie_processor = HTTPCookieProcessor(cookie_jar)
-        self.redirect_handler = XMLRPCRedirectHandler()
-        self.opener = build_opener(
-            self.cookie_processor, self.redirect_handler)
-
-    def setCookie(self, cookie_str):
-        """Set a cookie for the transport to use in future connections."""
-        name, value = cookie_str.split('=')
-        cookie = Cookie(
-            version=0, name=name, value=value,
-            port=None, port_specified=False,
-            domain=self.host, domain_specified=True,
-            domain_initial_dot=None,
-            path='', path_specified=False,
-            secure=False, expires=False, discard=None,
-            comment=None, comment_url=None, rest=None)
-        self.cookie_processor.cookiejar.set_cookie(cookie)
-
-    def request(self, host, handler, request_body, verbose=0):
-        """Make an XMLRPC request.
-
-        Uses the configured proxy server to make the connection.
-        """
-        url = urlunparse((self.scheme, host, handler, '', '', ''))
-        headers = {'Content-type': 'text/xml'}
-        request = Request(url, request_body, headers)
-        try:
-            response = self.parse_response(self.opener.open(request))
-        except HTTPError as he:
-            raise xmlrpclib.ProtocolError(
-                request.get_full_url(), he.code, he.msg, he.hdrs)
-        return response
+def fetch_buildbot_json(options, path, params=None):
+    """Fetch data from buildbot's JSON status views."""
+    buildbot_endpoint = urljoin(options.buildbot, 'json/')
+    response = requests.get(urljoin(buildbot_endpoint, path), params)
+    response.raise_for_status()
+    return response.json()
+
+
+def get_reasons_for_running_builds(options):
+    reasons = {}
+    builders_info = fetch_buildbot_json(options, 'builders')
+    for builder_name in builders_info:
+        current_builds = builders_info[builder_name]['currentBuilds']
+        if current_builds:
+            builds_info = fetch_buildbot_json(
+                options, 'builders/%s/builds' % quote(builder_name),
+                {'select': current_builds})
+            reasons[builder_name] = [
+                builds_info[str(build_id)]['reason']
+                for build_id in current_builds]
+        else:
+            reasons[builder_name] = []
+    return reasons
 
 
 def main():
@@ -305,19 +239,15 @@
             "--db-devel-local-branch %s is not a local directory."
             % options.db_devel_local_branch)
 
-    buildbot_endpoint = urljoin(options.buildbot, 'xmlrpc')
-    buildbot = xmlrpclib.ServerProxy(
-        buildbot_endpoint, UrlLib2Transport(buildbot_endpoint))
-
     revision, result = check_builder_and_push_to_stable(
-        buildbot, options.builder, options.devel_branch,
+        options.builder, options.devel_branch,
         options.stable_branch, options)
 
     db_revision, db_result = check_builder_and_push_to_stable(
-        buildbot, options.db_builder, options.db_devel_local_branch,
+        options.db_builder, options.db_devel_local_branch,
         options.db_stable_branch, options)
 
-    current_build_reasons = buildbot.getReasonsForRunningBuilds()
+    current_build_reasons = get_reasons_for_running_builds(options)
 
     # If both builds are successful, or they have received a [testfix]
     # landing, we should be in normal mode.
@@ -339,8 +269,7 @@
     return 0
 
 
-def check_builder_and_push_to_stable(buildbot, builder,
-                                     local_devel_branch,
+def check_builder_and_push_to_stable(builder, local_devel_branch,
                                      stable_branch, options):
     """Check a builder and push to the stable branch in case of success.
 
@@ -349,19 +278,22 @@
     # Get the last 10 builds info. We request more build info for the case
     # where the last one actually failed on checkout. We need a couple of
     # backlog to see what is the last revision actually tested.
-    latest_build_infos = buildbot.getLastBuilds(builder, 10)
-
-    # They are sorted older first.
-    (builder, build_number, build_start, build_end,
-        branch_name, revision, result, text, reasons) = latest_build_infos.pop()
-
+    latest_build_infos = fetch_buildbot_json(
+        options, 'builders/%s/builds' % quote(builder),
+        {'select': list(range(-1, -11, -1))})
+
+    latest_build = latest_build_infos['-1']
+    result = Results[latest_build['results']]
     if options.force_result is not None:
         result = options.force_result
+    text = latest_build['text']
+    properties = {name: value for name, value, _ in latest_build['properties']}
 
+    revision = properties.get('got_revision')
     try:
         revision = int(revision)
     except ValueError:
-        if revision in ('None', ''):
+        if revision in (None, ''):
             # The last build failed before checkout. Find the latest revision
             # tested before that.
             if result not in ('failure', 'exception'):
@@ -371,11 +303,14 @@
                     'Builder %s at %s failed to checkout branch '
                     'during last build: %s' % (
                     builder, options.buildbot, " ".join(text)))
-            for info in reversed(latest_build_infos):
+            for selector in range(-10, -1):
                 try:
-                    return int(info[4]), result
-                except ValueError:
-                    # Ignore invalid revision.
+                    build = latest_build_infos[str(selector)]
+                    properties = {
+                        name: value for name, value, _ in build['properties']}
+                    return int(properties['got_revision']), result
+                except (KeyError, ValueError):
+                    # Ignore invalid revision or missing build.
                     pass
             else:
                 print(