[Piglit] [PATCH 2/2] framework/log: add an HTTP logger

Dylan Baker baker.dylan.c at gmail.com
Tue Sep 22 13:11:21 PDT 2015


This looks good, thanks for making changes.

I have one rather lengthy comment below, but I'm fine with this landing
pretty much as-is, once my question about the previous patch is
answered.

On Tue, Sep 22, 2015 at 05:22:24PM +0100, Thomas Wood wrote:
> Add an HTTP logger so that it is possible to monitor the progress and
> status of piglit remotely through a webserver. The webserver runs on
> port 8080 by default and can be configured to run on a different port by
> setting the "port" key in the "http" section of the configuration file.
> The server responds to requests for /summary, which provides a summary
> of the current state in JSON format.
> 
> v2: Add appropriate locking when reading state data for the web server
>     Stop the server after the request for the final results
>     Remove the full results page
>     Add a configuration option to set the server port
> 
> Signed-off-by: Thomas Wood <thomas.wood at intel.com>
> ---
>  framework/log.py          | 76 +++++++++++++++++++++++++++++++++++++++++++++++
>  framework/programs/run.py |  2 +-
>  2 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/framework/log.py b/framework/log.py
> index 6d5a31c..868561e 100644
> --- a/framework/log.py
> +++ b/framework/log.py
> @@ -32,6 +32,14 @@ import abc
>  import itertools
>  import threading
>  import collections
> +import ConfigParser
> +from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler
> +try:
> +    import simplejson as json
> +except ImportError:
> +    import json
> +
> +from framework.core import PIGLIT_CONFIG
>  
>  __all__ = ['LogManager']
>  
> @@ -252,6 +260,68 @@ class DummyLog(BaseLog):
>          pass
>  
>  
> +class HTTPLogServer(threading.Thread):
> +    class RequestHandler(BaseHTTPRequestHandler):
> +        INDENT = 4
> +
> +        def do_GET(self):
> +            if self.path == "/summary":
> +                self.send_response(200)
> +                self.end_headers()
> +                with self.server.state_lock:
> +                    status = {
> +                        "complete": self.server.state["complete"],
> +                        "running" : self.server.state["running"],
> +                        "total"   : self.server.state["total"],
> +                        "results" : self.server.state["summary"],
> +                    }
> +                self.wfile.write(json.dumps(status, indent=self.INDENT))
> +            else:
> +                self.send_response(404)
> +                self.end_headers()
> +
> +    def __init__(self, state, state_lock):
> +        super(HTTPLogServer, self).__init__()
> +        try:
> +            port = PIGLIT_CONFIG.getint("http", "port")
> +        except (ConfigParser.NoOptionError, ConfigParser.NoSectionError):
> +            port = 8080

The PIGLIT_CONFIG constant is actually a subclass of SafeConfigParser.
(Actually, this example shows me that I should extend the safe_get
method a little). I've sent out a patch to add a fallback option which
would make this even simpler.

You could implement this as:
try:
    port = int(PIGLIT_CONFIG.safe_get('http', 'port'))
except TypeError:
    port = 8080

This isn't much better, but at least it avoids importing ConfigParser.


With the patch I sent to the list it would be:
port = int(PIGLIT_CONFIG.safe_get('http', 'port', fallback=8080))

I'm okay with you not taking any of these suggestions. If you choose not
to, would you be so kind as to add a 'FIXME: use safe_get here'?

> +        self._httpd = HTTPServer(("", port), HTTPLogServer.RequestHandler)
> +        self._httpd.state = state
> +        self._httpd.state_lock = state_lock
> +
> +    def run(self):
> +        while True:
> +            with self._httpd.state_lock:
> +                # stop handling requests after the request for the final results
> +                if self._httpd.state["complete"] == self._httpd.state["total"]:
> +                    break;
> +            self._httpd.handle_request()
> +
> +
> +class HTTPLog(BaseLog):
> +    """ A Logger that serves status information over http """
> +
> +    def __init__(self, state, state_lock):
> +        super(HTTPLog, self).__init__(state, state_lock)
> +        self._name = None
> +
> +    def start(self, name):
> +        with self._LOCK:
> +            self._name = name
> +            self._state['running'].append(self._name)
> +
> +    def log(self, status):
> +        with self._LOCK:
> +            self._state['running'].remove(self._name)
> +            self._state['complete'] += 1
> +            assert status in self.SUMMARY_KEYS
> +            self._state['summary'][status] += 1
> +
> +    def summary(self):
> +        pass
> +
> +
>  class LogManager(object):
>      """ Creates new log objects
>  
> @@ -274,6 +344,7 @@ class LogManager(object):
>          'quiet': QuietLog,
>          'verbose': VerboseLog,
>          'dummy': DummyLog,
> +        'http': HTTPLog,
>      }
>  
>      def __init__(self, logger, total):
> @@ -288,6 +359,11 @@ class LogManager(object):
>          }
>          self._state_lock = threading.Lock()
>  
> +        # start the http server for http logger
> +        if logger == 'http':
> +            self.log_server = HTTPLogServer(self._state, self._state_lock)
> +            self.log_server.start()
> +
>      def get(self):
>          """ Return a new log instance """
>          return self._log(self._state, self._state_lock)
> diff --git a/framework/programs/run.py b/framework/programs/run.py
> index bf8cc10..da99ac6 100644
> --- a/framework/programs/run.py
> +++ b/framework/programs/run.py
> @@ -158,7 +158,7 @@ def _run_parser(input_):
>      log_parser.add_argument("-l", "--log-level",
>                              dest="log_level",
>                              action="store",
> -                            choices=['quiet', 'verbose', 'dummy'],
> +                            choices=['quiet', 'verbose', 'dummy', 'http'],
>                              default='quiet',
>                              help="Set the logger verbosity level")
>      parser.add_argument("--test-list",
> -- 
> 1.9.1
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150922/be56a274/attachment.sig>


More information about the Piglit mailing list