[Piglit] [PATCH 2/2] framework/log: add an HTTP logger
Thomas Wood
thomas.wood at intel.com
Wed Sep 23 06:54:27 PDT 2015
On 22 September 2015 at 21:11, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> 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've sent a new version of the patch using the new fallback parameter.
>
> 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
Now that commit b365367 (framework: replace TestResult dict with an
object) has been pushed, status is no longer a string. This means it
needs to be converted back to a string to allow json.dumps to work
correctly. This is fixed in the updated patch.
>> +
>> + 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
More information about the Piglit
mailing list