build-requests should use conform to JSON API format
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Jun 2015 21:31:13 +0000 (21:31 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Jun 2015 21:31:13 +0000 (21:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=146375

Reviewed by Stephanie Lewis.

Instead of returning single dictionary that maps root set id to a dictionary of repository names
to revisions, timestamps, simply return root sets and roots "rows" or "objects" as defined in
JSON API (http://jsonapi.org/). This API change makes it easier to resolve the bug 146374 and
matches what we do in /api/test-groups.

Also add the support for /api/build-requests/?id=<id> to fetch the build request with <id>.
This is useful for debugging purposes.

* public/api/build-requests.php:
(main): Added the support for $_GET['id']. Also return "rootSets" and "roots".
(update_builds): Extracted from main.

* public/include/build-requests-fetcher.php:
(BuildRequestFetcher::fetch_request): Added. Used for /api/build-requests/?id=<id>.
(BuildRequestFetcher::results_internal): Always call fetch_roots_for_set_if_needed.
(BuildRequestFetcher::fetch_roots_for_set_if_needed): Renamed from fetch_roots_for_set.
Moved the logic to exit early when the root set had already been fetched here.

* public/v2/analysis.js:
(App.TestGroup._fetchTestResults): Fixed the bug that test groups without any successful results
won't be shown.

* tools/pull-os-versions.py:
(main):
(setup_auth): Moved to util.py

* tools/sync-with-buildbot.py:
(main): Replaced a bunch of perf dashboard related options by --server-config-json.
(update_and_fetch_build_requests): No longer takes build_request_auth since that's now taken care
of by setup_auth.
(organize_root_sets_by_id_and_repository_names): Added. Builds the old rootsSets directory based
on "roots" and "rootSets" dictionaries returned by /api/build-requests.
(config_for_request): Fixed a bug that the script blows up when the build request is missing
the repository specified in the configuration. This tolerance is necessary when a new repository
dependency is added but we want to run A/B tests for old builds without the dependency.
(fetch_json): No longer takes auth.

* tools/util.py:
(setup_auth): Moved from pull-os-versions.py to be shared with sync-with-buildbot.py.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@186033 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/api/build-requests.php
Websites/perf.webkit.org/public/include/build-requests-fetcher.php
Websites/perf.webkit.org/public/v2/analysis.js
Websites/perf.webkit.org/tools/pull-os-versions.py
Websites/perf.webkit.org/tools/sync-with-buildbot.py
Websites/perf.webkit.org/tools/util.py

index 4ac5d71..66a6d11 100644 (file)
@@ -1,3 +1,50 @@
+2015-06-27  Ryosuke Niwa  <rniwa@webkit.org>
+
+        build-requests should use conform to JSON API format
+        https://bugs.webkit.org/show_bug.cgi?id=146375
+
+        Reviewed by Stephanie Lewis.
+
+        Instead of returning single dictionary that maps root set id to a dictionary of repository names
+        to revisions, timestamps, simply return root sets and roots "rows" or "objects" as defined in
+        JSON API (http://jsonapi.org/). This API change makes it easier to resolve the bug 146374 and
+        matches what we do in /api/test-groups.
+
+        Also add the support for /api/build-requests/?id=<id> to fetch the build request with <id>.
+        This is useful for debugging purposes.
+
+        * public/api/build-requests.php:
+        (main): Added the support for $_GET['id']. Also return "rootSets" and "roots".
+        (update_builds): Extracted from main.
+
+        * public/include/build-requests-fetcher.php:
+        (BuildRequestFetcher::fetch_request): Added. Used for /api/build-requests/?id=<id>.
+        (BuildRequestFetcher::results_internal): Always call fetch_roots_for_set_if_needed.
+        (BuildRequestFetcher::fetch_roots_for_set_if_needed): Renamed from fetch_roots_for_set.
+        Moved the logic to exit early when the root set had already been fetched here.
+
+        * public/v2/analysis.js:
+        (App.TestGroup._fetchTestResults): Fixed the bug that test groups without any successful results
+        won't be shown.
+
+        * tools/pull-os-versions.py:
+        (main):
+        (setup_auth): Moved to util.py
+
+        * tools/sync-with-buildbot.py:
+        (main): Replaced a bunch of perf dashboard related options by --server-config-json.
+        (update_and_fetch_build_requests): No longer takes build_request_auth since that's now taken care
+        of by setup_auth.
+        (organize_root_sets_by_id_and_repository_names): Added. Builds the old rootsSets directory based
+        on "roots" and "rootSets" dictionaries returned by /api/build-requests.
+        (config_for_request): Fixed a bug that the script blows up when the build request is missing
+        the repository specified in the configuration. This tolerance is necessary when a new repository
+        dependency is added but we want to run A/B tests for old builds without the dependency.
+        (fetch_json): No longer takes auth.
+
+        * tools/util.py:
+        (setup_auth): Moved from pull-os-versions.py to be shared with sync-with-buildbot.py.
+
 2015-06-23  Ryosuke Niwa  <rniwa@webkit.org>
 
         Build fix. A/B testing is broken when continuous builders report revisions out of order.
index bcad292..4389ef1 100644 (file)
@@ -3,54 +3,63 @@
 require_once('../include/json-header.php');
 require_once('../include/build-requests-fetcher.php');
 
-function main($path, $post_data) {
-    if (count($path) < 1 || count($path) > 2)
+function main($id, $path, $post_data) {
+    if (!$id && (count($path) < 1 || count($path) > 2))
         exit_with_error('InvalidRequest');
 
     $db = new Database;
     if (!$db->connect())
         exit_with_error('DatabaseConnectionFailure');
 
-    $triggerable_query = array('name' => array_get($path, 0));
-    $triggerable = $db->select_first_row('build_triggerables', 'triggerable', $triggerable_query);
-    if (!$triggerable)
-        exit_with_error('TriggerableNotFound', $triggerable_query);
-
     $report = $post_data ? json_decode($post_data, true) : array();
     $updates = array_get($report, 'buildRequestUpdates');
     if ($updates) {
         verify_slave($db, $report);
-
-        $db->begin_transaction();
-        foreach ($updates as $id => $info) {
-            $id = intval($id);
-            $status = $info['status'];
-            $url = array_get($info, 'url');
-            if ($status == 'failedIfNotCompleted') {
-                $db->query_and_get_affected_rows('UPDATE build_requests SET (request_status, request_url) = ($1, $2)
-                    WHERE request_id = $3 AND request_status != $4', array('failed', $url, $id, 'completed'));
-            } else {
-                if (!in_array($status, array('pending', 'scheduled', 'running', 'failed', 'completed'))) {
-                    $db->rollback_transaction();
-                    exit_with_error('UnknownBuildRequestStatus', array('buildRequest' => $id, 'status' => $status));
-                }
-                $db->update_row('build_requests', 'request', array('id' => $id), array('status' => $status, 'url' => $url));
-            } 
-        }
-        $db->commit_transaction();
+        update_builds($db, $updates);
     }
 
     $requests_fetcher = new BuildRequestsFetcher($db);
-    $requests_fetcher->fetch_incomplete_requests_for_triggerable($triggerable['triggerable_id']);
+
+    if ($id)
+        $requests_fetcher->fetch_request($id);
+    else {
+        $triggerable_query = array('name' => array_get($path, 0));
+        $triggerable = $db->select_first_row('build_triggerables', 'triggerable', $triggerable_query);
+        if (!$triggerable)
+            exit_with_error('TriggerableNotFound', $triggerable_query);
+        $requests_fetcher->fetch_incomplete_requests_for_triggerable($triggerable['triggerable_id']);
+    }
 
     exit_with_success(array(
         'buildRequests' => $requests_fetcher->results_with_resolved_ids(),
-        'rootSets' => $requests_fetcher->root_sets_by_id(),
+        'rootSets' => $requests_fetcher->root_sets(),
+        'roots' => $requests_fetcher->roots(),
         'updates' => $updates,
     ));
 }
 
-main(array_key_exists('PATH_INFO', $_SERVER) ? explode('/', trim($_SERVER['PATH_INFO'], '/')) : array(),
+function update_builds($db, $updates) {
+    $db->begin_transaction();
+    foreach ($updates as $id => $info) {
+        $id = intval($id);
+        $status = $info['status'];
+        $url = array_get($info, 'url');
+        if ($status == 'failedIfNotCompleted') {
+            $db->query_and_get_affected_rows('UPDATE build_requests SET (request_status, request_url) = ($1, $2)
+                WHERE request_id = $3 AND request_status != $4', array('failed', $url, $id, 'completed'));
+        } else {
+            if (!in_array($status, array('pending', 'scheduled', 'running', 'failed', 'completed'))) {
+                $db->rollback_transaction();
+                exit_with_error('UnknownBuildRequestStatus', array('buildRequest' => $id, 'status' => $status));
+            }
+            $db->update_row('build_requests', 'request', array('id' => $id), array('status' => $status, 'url' => $url));
+        }
+    }
+    $db->commit_transaction();
+}
+
+main(array_get($_GET, 'id'),
+    array_key_exists('PATH_INFO', $_SERVER) ? explode('/', trim($_SERVER['PATH_INFO'], '/')) : array(),
     file_get_contents("php://input"));
 
 ?>
index f7b0d63..f724e8b 100644 (file)
@@ -30,6 +30,10 @@ class BuildRequestsFetcher {
             ORDER BY request_created_at, request_group, request_order', array($triggerable_id));
     }
 
+    function fetch_request($request_id) {
+        $this->rows = $this->db->select_rows('build_requests', 'request', array('id' => $request_id));
+    }
+
     function has_results() { return is_array($this->rows); }
     function results() { return $this->results_internal(false); }
     function results_with_resolved_ids() { return $this->results_internal(true); }
@@ -51,8 +55,7 @@ class BuildRequestsFetcher {
             $platform_id = $row['request_platform'];
             $root_set_id = $row['request_root_set'];
 
-            if (!array_key_exists($root_set_id, $this->root_sets_by_id))
-                $this->root_sets_by_id[$root_set_id] = $this->fetch_roots_for_set($root_set_id, $resolve_ids);
+            $this->fetch_roots_for_set_if_needed($root_set_id, $resolve_ids);
 
             array_push($requests, array(
                 'id' => $row['request_id'],
@@ -71,10 +74,6 @@ class BuildRequestsFetcher {
         return $requests;
     }
 
-    function root_sets_by_id() {
-        return $this->root_sets_by_id;
-    }
-
     function root_sets() {
         return $this->root_sets;
     }
@@ -83,12 +82,14 @@ class BuildRequestsFetcher {
         return $this->roots;
     }
 
-    private function fetch_roots_for_set($root_set_id, $resolve_ids) {
+    private function fetch_roots_for_set_if_needed($root_set_id, $resolve_ids) {
+        if (array_key_exists($root_set_id, $this->root_sets_by_id))
+            return;
+
         $root_rows = $this->db->query_and_fetch_all('SELECT *
             FROM roots, commits LEFT OUTER JOIN repositories ON commit_repository = repository_id
             WHERE root_commit = commit_id AND root_set = $1', array($root_set_id));
 
-        $roots = array();
         $root_ids = array();
         foreach ($root_rows as $row) {
             $repository = $row['repository_id'];
@@ -96,13 +97,17 @@ class BuildRequestsFetcher {
             $commit_time = $row['commit_time'];
             $root_id = $root_set_id . '-' . $repository;
             array_push($root_ids, $root_id);
-            array_push($this->roots, array('id' => $root_id, 'repository' => $repository, 'revision' => $revision));
-            $roots[$resolve_ids ? $row['repository_name'] : $row['repository_id']] = array(
-                'revision' => $revision, 'time' => Database::to_js_time($commit_time));
+            $repository_id = $resolve_ids ? $row['repository_name'] : $row['repository_id'];
+            array_push($this->roots, array(
+                'id' => $root_id,
+                'repository' => $repository_id,
+                'revision' => $revision,
+                'time' => Database::to_js_time($commit_time)));
         }
-        array_push($this->root_sets, array('id' => $root_set_id, 'roots' => $root_ids));
 
-        return $roots;
+        $this->root_sets_by_id[$root_set_id] = TRUE;
+
+        array_push($this->root_sets, array('id' => $root_set_id, 'roots' => $root_ids));
     }
 }
 
index aa75907..d6b8803 100644 (file)
@@ -133,7 +133,7 @@ App.TestGroup = App.NameLabelModel.extend({
     _fetchTestResults: function ()
     {
         var task = this.get('task');
-        var platform = this.get('platform');
+        var platform = this.get('platform') || (task ? task.get('platform') : null)
         if (!task || !platform)
             return null;
         var self = this;
index 9dc7f97..17c5738 100755 (executable)
@@ -12,12 +12,7 @@ import urllib2
 from xml.dom.minidom import parseString as parseXmlString
 from util import submit_commits
 from util import text_content
-
-
-HTTP_AUTH_HANDLERS = {
-    'basic': urllib2.HTTPBasicAuthHandler,
-    'digest': urllib2.HTTPDigestAuthHandler,
-}
+from util import setup_auth
 
 
 def main(argv):
@@ -63,17 +58,6 @@ def main(argv):
         time.sleep(config['fetchInterval'])
 
 
-def setup_auth(server):
-    auth = server.get('auth')
-    if not auth:
-        return
-
-    password_manager = urllib2.HTTPPasswordMgr()
-    password_manager.add_password(realm=auth['realm'], uri=server['url'], user=auth['username'], passwd=auth['password'])
-    auth_handler = HTTP_AUTH_HANDLERS[auth['type']](password_manager)
-    urllib2.install_opener(urllib2.build_opener(auth_handler))
-
-
 def available_builds_from_command(repository_name, command, lines_to_ignore):
     try:
         output = subprocess.check_output(command, stderr=subprocess.STDOUT)
index 819e0b0..e410575 100755 (executable)
@@ -9,24 +9,30 @@ import time
 import urllib
 import urllib2
 
+from util import setup_auth
+
 
 def main():
     parser = argparse.ArgumentParser()
-    parser.add_argument('--build-requests-url', required=True, help='URL for the build requests JSON API; e.g. https://perf.webkit.org/api/build-requests/build.webkit.org/')
-    parser.add_argument('--build-requests-user', help='The username for Basic Authentication to access the build requests JSON API')
-    parser.add_argument('--build-requests-password', help='The password for Basic Authentication to access the build requests JSON API')
-    parser.add_argument('--slave-name', required=True, help='The slave name used to update the build requets status')
-    parser.add_argument('--slave-password', required=True, help='The slave password used to update the build requets status')
+    parser.add_argument('--triggerable', required=True, help='The name of the triggerable to process. e.g. build-webkit')
     parser.add_argument('--buildbot-url', required=True, help='URL for a buildbot builder; e.g. "https://build.webkit.org/"')
     parser.add_argument('--builder-config-json', required=True, help='The path to a JSON file that specifies which test and platform will be posted to which builder. '
         'The JSON should contain an array of dictionaries with keys "platform", "test", and "builder" '
         'with the platform name (e.g. mountainlion), the test path (e.g. ["Parser", "html5-full-render"]), and the builder name (e.g. Apple MountainLion Release (Perf)) as values.')
+    parser.add_argument('--server-config-json', required=True, help='The path to a JSON file that specifies the perf dashboard.')
+
     parser.add_argument('--lookback-count', type=int, default=10, help='The number of builds to look back when finding in-progress builds on the buildbot')
     parser.add_argument('--seconds-to-sleep', type=float, default=120, help='The seconds to sleep between iterations')
     args = parser.parse_args()
 
     configurations = load_config(args.builder_config_json, args.buildbot_url.strip('/'))
-    build_request_auth = {'user': args.build_requests_user, 'password': args.build_requests_password or ''} if args.build_requests_user else None
+
+    with open(args.server_config_json) as server_config_json:
+        server_config = json.load(server_config_json)
+        setup_auth(server_config['server'])
+
+    build_requests_url = server_config['server']['url'] + '/api/build-requests/' + args.triggerable
+
     request_updates = {}
     while True:
         request_updates.update(find_request_updates(configurations, args.lookback_count))
@@ -35,14 +41,21 @@ def main():
         else:
             print 'No updates...'
 
-        payload = {'buildRequestUpdates': request_updates, 'slaveName': args.slave_name, 'slavePassword': args.slave_password}
-        response = update_and_fetch_build_requests(args.build_requests_url, build_request_auth, payload)
-        root_sets = response.get('rootSets', {})
+        payload = {
+            'buildRequestUpdates': request_updates,
+            'slaveName': server_config['slave']['name'],
+            'slavePassword': server_config['slave']['password']}
+        response = update_and_fetch_build_requests(build_requests_url, payload)
         open_requests = response.get('buildRequests', [])
 
+        root_sets = organize_root_sets_by_id_and_repository_names(response.get('rootSets', {}), response.get('roots', []))
+
         for request in filter(lambda request: request['status'] == 'pending', open_requests):
             config = config_for_request(configurations, request)
-            if len(config['scheduledRequests']) < 1:
+            if not config:
+                print >> sys.stderr, "Failed to find the configuration for request %s: %s" % (str(request['id']), json.dumps(request))
+                continue
+            if config and len(config['scheduledRequests']) < 1:
                 print "Scheduling the build request %s..." % str(request['id'])
                 schedule_request(config, request, root_sets)
 
@@ -109,9 +122,9 @@ def find_request_updates(configurations, lookback_count):
     return request_updates
 
 
-def update_and_fetch_build_requests(build_requests_url, build_request_auth, payload):
+def update_and_fetch_build_requests(build_requests_url, payload):
     try:
-        response = fetch_json(build_requests_url, payload=json.dumps(payload), auth=build_request_auth)
+        response = fetch_json(build_requests_url, payload=json.dumps(payload))
         if response['status'] != 'OK':
             raise ValueError(response['status'])
         return response
@@ -132,15 +145,32 @@ def find_stale_request_updates(configurations, open_requests, requests_on_buildb
     return request_updates
 
 
-def schedule_request(config, request, root_sets):
-    roots = root_sets.get(request['rootSet'], {})
+def organize_root_sets_by_id_and_repository_names(root_sets, roots):
+    result = {}
+    root_by_id = {}
+    for root in roots:
+        root_by_id[root['id']] = root
+
+    for root_set in root_sets:
+        roots_by_repository = {}
+        for root_id in root_set['roots']:
+            root = root_by_id[root_id]
+            roots_by_repository[root['repository']] = root
+        result[root_set['id']] = roots_by_repository
+
+    return result
 
+
+def schedule_request(config, request, root_sets):
+    roots = root_sets[request['rootSet']]
     payload = {}
     for property_name, property_value in config['arguments'].iteritems():
         if not isinstance(property_value, dict):
             payload[property_name] = property_value
         elif 'root' in property_value:
-            payload[property_name] = roots[property_value['root']]['revision']
+            repository_name = property_value['root']
+            if repository_name in roots:
+                payload[property_name] = roots[repository_name]['revision']
         elif 'rootsExcluding' in property_value:
             excluded_roots = property_value['rootsExcluding']
             filtered_roots = {}
@@ -150,6 +180,7 @@ def schedule_request(config, request, root_sets):
             payload[property_name] = json.dumps(filtered_roots)
         else:
             print >> sys.stderr, "Failed to process an argument %s: %s" % (property_name, property_value)
+            return
     payload[config['buildRequestArgument']] = request['id']
 
     try:
@@ -166,10 +197,8 @@ def config_for_request(configurations, request):
     return None
 
 
-def fetch_json(url, auth={}, payload=None):
+def fetch_json(url, payload=None):
     request = urllib2.Request(url)
-    if auth:
-        request.add_header('Authorization', "Basic %s" % base64.encodestring('%s:%s' % (auth['user'], auth['password'])).rstrip('\n'))
     response = urllib2.urlopen(request, payload).read()
     try:
         return json.loads(response)
index 2449a34..86956b4 100644 (file)
@@ -34,3 +34,20 @@ def text_content(element):
         else:
             text += text_content(child)
     return text
+
+
+HTTP_AUTH_HANDLERS = {
+    'basic': urllib2.HTTPBasicAuthHandler,
+    'digest': urllib2.HTTPDigestAuthHandler,
+}
+
+
+def setup_auth(server):
+    auth = server.get('auth')
+    if not auth:
+        return
+
+    password_manager = urllib2.HTTPPasswordMgr()
+    password_manager.add_password(realm=auth['realm'], uri=server['url'], user=auth['username'], passwd=auth['password'])
+    auth_handler = HTTP_AUTH_HANDLERS[auth['type']](password_manager)
+    urllib2.install_opener(urllib2.build_opener(auth_handler))