Patchwork hgweb: don't try to wrap mod_wsgi loggers

login
register
mail settings
Submitter Ludovic Chabant
Date Sept. 29, 2020, 9:32 p.m.
Message ID <f8726d166fcaa0d690f6.1601415141@barjoland.chabant.com>
Download mbox | patch
Permalink /patch/47338/
State New
Headers show

Comments

Ludovic Chabant - Sept. 29, 2020, 9:32 p.m.
# HG changeset patch
# User Ludovic Chabant <ludovic@chabant.com>
# Date 1601413962 25200
#      Tue Sep 29 14:12:42 2020 -0700
# Node ID f8726d166fcaa0d690f6783146172758b2e14deb
# Parent  80bf7b1ada15622ea45b8ecc5647404f5acb2905
hgweb: don't try to wrap mod_wsgi loggers
Yuya Nishihara - Sept. 30, 2020, 11:23 a.m.
On Tue, 29 Sep 2020 14:32:21 -0700, Ludovic Chabant wrote:
> # HG changeset patch
> # User Ludovic Chabant <ludovic@chabant.com>
> # Date 1601413962 25200
> #      Tue Sep 29 14:12:42 2020 -0700
> # Node ID f8726d166fcaa0d690f6783146172758b2e14deb
> # Parent  80bf7b1ada15622ea45b8ecc5647404f5acb2905
> hgweb: don't try to wrap mod_wsgi loggers
> 
> diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
> --- a/mercurial/utils/procutil.py
> +++ b/mercurial/utils/procutil.py
> @@ -108,6 +108,10 @@
>          # The io.BufferedIOBase.write() contract guarantees that all data is
>          # written.
>          return stream
> +    if str(type(stream).__module__) == 'mod_wsgi':
> +	# WSGI loggers write all data. We couldn't even wrap them anyway since
> +	# they typically don't return the number of bytes written.
> +	return stream

Manuel, any comments?

It might be better to test isinstance(stream, io.RawIOBase). If the stream
is a RawIOBase, write() should return the number of bytes written.
Ludovic Chabant - Sept. 30, 2020, 4:16 p.m.
> It might be better to test isinstance(stream, io.RawIOBase). If the stream
> is a RawIOBase, write() should return the number of bytes written.

I'm not super familiar with Python C/native extension code, but it doesn't look to me like the mod_wsgi.Log type is inheriting from an io base class?
https://github.com/GrahamDumpleton/mod_wsgi/blob/develop/src/server/wsgi_logger.c#L538
Yuya Nishihara - Oct. 1, 2020, 10:03 a.m.
On Wed, 30 Sep 2020 09:16:27 -0700, Ludovic Chabant wrote:
> > It might be better to test isinstance(stream, io.RawIOBase). If the stream
> > is a RawIOBase, write() should return the number of bytes written.
> 
> I'm not super familiar with Python C/native extension code, but it doesn't look to me like the mod_wsgi.Log type is inheriting from an io base class?
> https://github.com/GrahamDumpleton/mod_wsgi/blob/develop/src/server/wsgi_logger.c#L538

Yep, that's my expectation. If the stream is a RawIOBase, work around the
partial write behavior. Otherwise do nothing since we don't know if the stream
follows the Py3's IO interface.

Patch

diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
--- a/mercurial/utils/procutil.py
+++ b/mercurial/utils/procutil.py
@@ -108,6 +108,10 @@ 
         # The io.BufferedIOBase.write() contract guarantees that all data is
         # written.
         return stream
+    if str(type(stream).__module__) == 'mod_wsgi':
+	# WSGI loggers write all data. We couldn't even wrap them anyway since
+	# they typically don't return the number of bytes written.
+	return stream
     # In general, the write() method of streams is free to write only part of
     # the data.
     return WriteAllWrapper(stream)