launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
> index 7325d9f..b245c92 100644
> --- a/lib/lp/services/webapp/publication.py
> +++ b/lib/lp/services/webapp/publication.py
> @@ -871,6 +872,8 @@ def tracelog(request, prefix, msg):
>      easier. ``prefix`` should be unique and contain no spaces, and
>      preferably a single character to save space.
>      """
> -    tracelog = ITraceLog(request, None)
> -    if tracelog is not None:
> -        tracelog.log('%s %s' % (prefix, six.ensure_str(msg, 'US-ASCII')))
> +    if not config.use_gunicorn:
> +        msg = '%s %s' % (prefix, six.ensure_str(msg, 'US-ASCII'))
> +        tracelog = ITraceLog(request, None)
> +        if tracelog is not None:
> +            tracelog.log(msg)

Hm, you're right.  For now I think logging a message at `DEBUG` would indeed be best, and we can see if we need to work out something better later on.

> diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
> index 9089c60..98c8149 100644
> --- a/lib/lp/testing/layers.py
> +++ b/lib/lp/testing/layers.py
> @@ -1794,6 +1798,14 @@ class LayerProcessController:
>              raise LayerIsolationError(
>                  "App server died in this test (status=%s):\n%s" % (
>                      cls.appserver.returncode, cls.appserver.stdout.read()))
> +        # Cleanup the app server's output buffer between tests.
> +        while True:
> +            # Read while we have something available at the stdout.
> +            r, w, e = select.select([cls.appserver.stdout], [], [], 0)
> +            if cls.appserver.stdout in r:
> +                cls.appserver.stdout.readline()

Oh, I guess I thought the FD was opened non-blocking so that `.read()` would only read as much as had been written so far.  If that's not the case, then fair enough.

> +            else:
> +                break
>          DatabaseLayer.force_dirty_database()
>  
>      @classmethod


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/396691
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:gunicorn-appserver-tests.