rebaseline_server: add --editable and --reload flags
authorepoger@google.com <epoger@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 15 Oct 2013 20:10:33 +0000 (20:10 +0000)
committerepoger@google.com <epoger@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 15 Oct 2013 20:10:33 +0000 (20:10 +0000)
This is a variation on the --browseonly flag we came up with in
https://codereview.chromium.org/24274003/#msg6 .

The long-lived "view only" server will probably be run like this:
server.py --export --reload 300

And when rebaselining you'll run your own server like this:
server.py --editable

(SkipBuildbotRuns)

R=jcgregorio@google.com

Review URL: https://codereview.chromium.org/27218005

git-svn-id: http://skia.googlecode.com/svn/trunk@11788 2bbb7eff-a529-9590-31e7-b0007b416f81

gm/rebaseline_server/results.py
gm/rebaseline_server/server.py
gm/rebaseline_server/static/loader.js
gm/rebaseline_server/static/view.html

index c0d3187..ac71156 100755 (executable)
@@ -16,6 +16,7 @@ import logging
 import os
 import re
 import sys
+import time
 
 # Imports from within Skia
 #
@@ -53,6 +54,13 @@ class Results(object):
     self._actual_builder_dicts = Results._get_dicts_from_root(actuals_root)
     self._expected_builder_dicts = Results._get_dicts_from_root(expected_root)
     self._combine_actual_and_expected()
+    self._timestamp = int(time.time())
+
+  def get_timestamp(self):
+    """Return the time at which this object was created, in seconds past epoch
+    (UTC).
+    """
+    return self._timestamp
 
   def get_results_of_type(self, type):
     """Return results of some/all tests (depending on 'type' parameter).
@@ -113,7 +121,12 @@ class Results(object):
     Returns:
       A meta-dictionary containing all the JSON dictionaries found within
       the directory tree, keyed by the builder name of each dictionary.
+
+    Raises:
+      IOError if root does not refer to an existing directory
     """
+    if not os.path.isdir(root):
+      raise IOError('no directory found at path %s' % root)
     meta_dict = {}
     for dirpath, dirnames, filenames in os.walk(root):
       for matching_filename in fnmatch.filter(filenames, pattern):
index bfc690b..a370379 100755 (executable)
@@ -19,6 +19,8 @@ import posixpath
 import re
 import shutil
 import sys
+import thread
+import time
 import urlparse
 
 # Imports from within Skia
@@ -38,6 +40,7 @@ import svn
 import results
 
 ACTUALS_SVN_REPO = 'http://skia-autogen.googlecode.com/svn/gm-actual'
+EXPECTATIONS_SVN_REPO = 'http://skia.googlecode.com/svn/trunk/expectations/gm'
 PATHSPLIT_RE = re.compile('/([^/]+)/(.+)')
 TRUNK_DIRECTORY = os.path.dirname(os.path.dirname(os.path.dirname(
     os.path.realpath(__file__))))
@@ -65,7 +68,8 @@ class Server(object):
   def __init__(self,
                actuals_dir=DEFAULT_ACTUALS_DIR,
                expectations_dir=DEFAULT_EXPECTATIONS_DIR,
-               port=DEFAULT_PORT, export=False):
+               port=DEFAULT_PORT, export=False, editable=True,
+               reload_seconds=0):
     """
     Args:
       actuals_dir: directory under which we will check out the latest actual
@@ -74,33 +78,60 @@ class Server(object):
                         must already be in that directory)
       port: which TCP port to listen on for HTTP requests
       export: whether to allow HTTP clients on other hosts to access this server
+      editable: whether HTTP clients are allowed to submit new baselines
+      reload_seconds: polling interval with which to check for new results;
+                      if 0, don't check for new results at all
     """
     self._actuals_dir = actuals_dir
     self._expectations_dir = expectations_dir
     self._port = port
     self._export = export
+    self._editable = editable
+    self._reload_seconds = reload_seconds
 
   def is_exported(self):
     """ Returns true iff HTTP clients on other hosts are allowed to access
     this server. """
     return self._export
 
-  def fetch_results(self):
-    """ Create self.results, based on the expectations in
-    self._expectations_dir and the latest actuals from skia-autogen.
+  def is_editable(self):
+    """ Returns true iff HTTP clients are allowed to submit new baselines. """
+    return self._editable
+
+  def reload_seconds(self):
+    """ Returns the result reload period in seconds, or 0 if we don't reload
+    results. """
+    return self._reload_seconds
 
-    TODO(epoger): Add a new --browseonly mode setting.  In that mode,
-    the gm-actuals and expectations will automatically be updated every few
-    minutes.  See discussion in https://codereview.chromium.org/24274003/ .
+  def _update_results(self):
+    """ Create or update self.results, based on the expectations in
+    self._expectations_dir and the latest actuals from skia-autogen.
     """
-    logging.info('Checking out latest actual GM results from %s into %s ...' % (
-        ACTUALS_SVN_REPO, self._actuals_dir))
+    logging.info('Updating actual GM results in %s from SVN repo %s ...' % (
+        self._actuals_dir, ACTUALS_SVN_REPO))
     actuals_repo = svn.Svn(self._actuals_dir)
     if not os.path.isdir(self._actuals_dir):
       os.makedirs(self._actuals_dir)
       actuals_repo.Checkout(ACTUALS_SVN_REPO, '.')
     else:
       actuals_repo.Update('.')
+
+    # We only update the expectations dir if the server was run with a nonzero
+    # --reload argument; otherwise, we expect the user to maintain her own
+    # expectations as she sees fit.
+    #
+    # TODO(epoger): Use git instead of svn to check out expectations, since
+    # the Skia repo is moving to git.
+    if self._reload_seconds:
+      logging.info('Updating expected GM results in %s from SVN repo %s ...' % (
+          self._expectations_dir, EXPECTATIONS_SVN_REPO))
+      expectations_repo = svn.Svn(self._expectations_dir)
+      if not os.path.isdir(self._expectations_dir):
+        os.makedirs(self._expectations_dir)
+        expectations_repo.Checkout(EXPECTATIONS_SVN_REPO, '.')
+      else:
+        expectations_repo.Update('.')
+
     logging.info(
         'Parsing results from actuals in %s and expectations in %s ...' % (
         self._actuals_dir, self._expectations_dir))
@@ -108,12 +139,26 @@ class Server(object):
       actuals_root=self._actuals_dir,
       expected_root=self._expectations_dir)
 
+  def _result_reloader(self):
+    """ If --reload argument was specified, reload results at the appropriate
+    interval.
+    """
+    while self._reload_seconds:
+      time.sleep(self._reload_seconds)
+      with self.results_lock:
+        self._update_results()
+
   def run(self):
-    self.fetch_results()
+    self._update_results()
+    self.results_lock = thread.allocate_lock()
+    thread.start_new_thread(self._result_reloader, ())
+
     if self._export:
       server_address = ('', self._port)
-      logging.warning('Running in "export" mode. Users on other machines will '
-                      'be able to modify your GM expectations!')
+      if self._editable:
+        logging.warning('Running with combination of "export" and "editable" '
+                        'flags.  Users on other machines will '
+                        'be able to modify your GM expectations!')
     else:
       server_address = ('127.0.0.1', self._port)
     http_server = BaseHTTPServer.HTTPServer(server_address, HTTPRequestHandler)
@@ -162,17 +207,29 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
       # to refer to the Server object, make Server a subclass of
       # HTTPServer, and then it could be available to the handler via
       # the handler's .server instance variable.
-      response_dict = _SERVER.results.get_results_of_type(type)
+
+      with _SERVER.results_lock:
+        response_dict = _SERVER.results.get_results_of_type(type)
+        time_updated = _SERVER.results.get_timestamp()
       response_dict['header'] = {
+        # Timestamps:
+        # 1. when this data was last updated
+        # 2. when the caller should check back for new data (if ever)
+        #
+        # We only return these timestamps if the --reload argument was passed;
+        # otherwise, we have no idea when the expectations were last updated
+        # (we allow the user to maintain her own expectations as she sees fit).
+        'timeUpdated': time_updated if _SERVER.reload_seconds() else None,
+        'timeNextUpdateAvailable': (
+            (time_updated+_SERVER.reload_seconds()) if _SERVER.reload_seconds()
+            else None),
+
         # Hash of testData, which the client must return with any edits--
         # this ensures that the edits were made to a particular dataset.
-        'data-hash': str(hash(repr(response_dict['testData']))),
+        'dataHash': str(hash(repr(response_dict['testData']))),
 
         # Whether the server will accept edits back.
-        # TODO(epoger): Not yet implemented, so hardcoding to False;
-        # once we implement the 'browseonly' mode discussed in
-        # https://codereview.chromium.org/24274003/#msg6 , this value will vary.
-        'isEditable': False,
+        'isEditable': _SERVER.is_editable(),
 
         # Whether the service is accessible from other hosts.
         'isExported': _SERVER.is_exported(),
@@ -180,6 +237,7 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
       self.send_json_dict(response_dict)
     except:
       self.send_error(404)
+      raise
 
   def do_GET_static(self, path):
     """ Handle a GET request for a file under the 'static' directory.
@@ -259,6 +317,9 @@ def main():
                           'actual GM results. If this directory does not '
                           'exist, it will be created. Defaults to %(default)s'),
                     default=DEFAULT_ACTUALS_DIR)
+  parser.add_argument('--editable', action='store_true',
+                      help=('TODO(epoger): NOT YET IMPLEMENTED.  '
+                            'Allow HTTP clients to submit new baselines.'))
   parser.add_argument('--expectations-dir',
                     help=('Directory under which to find GM expectations; '
                           'defaults to %(default)s'),
@@ -268,15 +329,24 @@ def main():
                             'on localhost, allow HTTP clients on other hosts '
                             'to access this server.  WARNING: doing so will '
                             'allow users on other hosts to modify your '
-                            'GM expectations!'))
+                            'GM expectations, if combined with --editable.'))
   parser.add_argument('--port', type=int,
                       help=('Which TCP port to listen on for HTTP requests; '
                             'defaults to %(default)s'),
                       default=DEFAULT_PORT)
+  parser.add_argument('--reload', type=int,
+                      help=('How often (a period in seconds) to update the '
+                            'results.  If specified, both EXPECTATIONS_DIR and '
+                            'ACTUAL_DIR will be updated.  '
+                            'By default, we do not reload at all, and you '
+                            'must restart the server to pick up new data.'),
+                      default=0)
   args = parser.parse_args()
   global _SERVER
-  _SERVER = Server(expectations_dir=args.expectations_dir,
-                   port=args.port, export=args.export)
+  _SERVER = Server(actuals_dir=args.actuals_dir,
+                   expectations_dir=args.expectations_dir,
+                   port=args.port, export=args.export, editable=args.editable,
+                   reload_seconds=args.reload)
   _SERVER.run()
 
 if __name__ == '__main__':
index 46d28fc..80f84c6 100644 (file)
@@ -31,7 +31,8 @@ Loader.filter(
 
 Loader.controller(
   'Loader.Controller',
-  function($scope, $http, $filter, $location) {
+    function($scope, $http, $filter, $location) {
+    $scope.windowTitle = "Loading GM Results...";
     var resultsToLoad = $location.search().resultsToLoad;
     $scope.loadingMessage = "Loading results of type '" + resultsToLoad +
         "', please wait...";
@@ -44,6 +45,7 @@ Loader.controller(
         $scope.categories = data.categories;
         $scope.testData = data.testData;
         $scope.sortColumn = 'test';
+        $scope.showTodos = true;
 
         for (var i = 0; i < $scope.testData.length; i++) {
           $scope.testData[i].index = i;
@@ -59,11 +61,13 @@ Loader.controller(
 
         $scope.updateResults();
         $scope.loadingMessage = "";
+        $scope.windowTitle = "Current GM Results";
       }
     ).error(
       function(data, status, header, config) {
         $scope.loadingMessage = "Failed to load results of type '"
             + resultsToLoad + "'";
+        $scope.windowTitle = "Failed to Load GM Results";
       }
     );
 
@@ -109,6 +113,11 @@ Loader.controller(
       $scope.areUpdatesPending = true;
     }
 
+    $scope.localTimeString = function(secondsPastEpoch) {
+      var d = new Date(secondsPastEpoch * 1000);
+      return d.toString();
+    }
+
     $scope.updateResults = function() {
       $scope.displayLimit = $scope.displayLimitPending;
       // TODO(epoger): Every time we apply a filter, AngularJS creates
index 0451b65..9f0158d 100644 (file)
@@ -1,19 +1,14 @@
 <!DOCTYPE html>
 
-<html ng-app="Loader">
+<html ng-app="Loader" ng-controller="Loader.Controller">
 
 <head>
-  <title>Current GM Results</title>
+  <title ng-bind="windowTitle"></title>
   <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.1.5/angular.js"></script>
   <script src="loader.js"></script>
 </head>
 
 <body>
-  <div ng-controller="Loader.Controller">
-
-  <!-- TODO(epoger): Add some indication of how old the
-  expected/actual data is -->
-
   <em>
     {{loadingMessage}}
   </em>
@@ -24,6 +19,9 @@
       WARNING!  These results are editable and exported, so any user
       who can connect to this server over the network can modify them.
     </div>
+    <div ng-hide="!(header.timeUpdated)">
+      Results current as of {{localTimeString(header.timeUpdated)}}
+    </div>
   <table border="1">
     <tr>
       <th colspan="2">
   </table>
 
     <p>
-      TODO(epoger): Add ability to filter builder and test names
-      (using a free-form text field, with partial string match)
-      <br>
-      TODO(epoger): Add more columns, such as pixel diffs, notes/bugs,
-      ignoreFailure boolean
-      <br>
-      TODO(epoger): Improve the column sorting, as per
-      <a href="http://jsfiddle.net/vojtajina/js64b/14/">
-        http://jsfiddle.net/vojtajina/js64b/14/
-      </a>
-      <br>
-      TODO(epoger): Right now, if you change which column is used to
-      sort the data, the column widths may fluctuate based on the
-      longest string <i>currently visible</i> within the top {{displayLimit}}
-      results.  Can we fix the column widths to be wide enough to hold
-      any result, even the currently hidden results?
+      TODO(epoger):
+      <input type="checkbox" name="showTodosCheckbox" value="true"
+             ng-checked="showTodos == true"
+             ng-click="showTodos = !showTodos">
+      show
+      <ul ng-hide="!showTodos">
+        <li>
+          Implement editing of results (we have added the --editable
+          flag to the server, but it&#39;s not fully implemented yet).
+          <div ng-hide="!header.isEditable">
+            Currently selected items are: {{selectedItems}}
+          </div>
+        </li><li>
+          If server was run with --reload flag, automatically check for
+          new results and tell the user when new results are available
+          (the user can reload the page if he wants to see them).
+        </li><li>
+          Add ability to filter builder and test names
+          (using a free-form text field, with partial string match)
+        </li><li>
+          Add more columns, such as pixel diffs, notes/bugs,
+          ignoreFailure boolean
+        </li><li>
+          Improve the column sorting, as per
+          <a href="http://jsfiddle.net/vojtajina/js64b/14/">
+            http://jsfiddle.net/vojtajina/js64b/14/
+          </a>
+        </li><li>
+          Right now, if you change which column is used to
+          sort the data, the column widths may fluctuate based on the
+          longest string <i>currently visible</i> within the top {{displayLimit}}
+          results.  Can we fix the column widths to be wide enough to hold
+          any result, even the currently hidden results?
+        </li>
+      </ul>
       <p>
       Found {{filteredTestData.length}} matches, and displaying the first
       {{displayLimit}}: <br>
           </th>
           <th ng-hide="!header.isEditable">
             <!-- item-selection checkbox column -->
-            {{selectedItems}}  <!-- TODO(epoger): temporary debug output -->
           </th>
         </tr>
         <tr ng-repeat="result in limitedTestData">
         </tr>
       </table>
   </div>
-  </div>
 
   <!-- TODO(epoger): Can we get the base URLs (commondatastorage and
        issues list) from