[Web-SIG] wsgiref problems & quibbles

[Phillip J. Eby]

At 11:54 PM 5/3/2006 -0700, Titus Brown wrote:
>Hi, Phillip,
>
>so I threw together a few WSGI apps recently, and used wsgiref to serve
>them.  I ran into two problems, and had a few quibbles to raise as well.
>
>===
>
>First, a patch to fix the existing unit tests is attached.

Thanks.


>===
>
>Second, there's a bug in handlers.BaseHandler.finish_response, line 132.
>The line:
>
>         if not self.result_is_file() and not self.sendfile():
>
>should read
>
>         if not self.result_is_file() or not self.sendfile():
>                                      ^^
>
>Otherwise use of wsgi.file_wrapper fails to return any data.

This has been changed in the SVN version now; thanks.


>I also can't find any place in the code where 'sendfile' is called to
>actually send the file.  Should that 'if' statement have an 'else'??

No.  See the docstring for the 'sendfile()' method.  'self.sendfile()' is 
supposed to send the file (if it can) and return True, otherwise False if 
it can't actually send the file.  The default implementation returns False.


>And now for the few miscellaneous quibbles ;), mostly in re WSGIServer:
>
>   * the __init__ function is inherited, ultimately, from
>         SocketServer.TCPServer via BaseHTTPServer.  This gives it three
>         uglification problems:
>
>           - the host, port are passed in as a tuple;
>           - the request handler must be explicitly specified;
>           - the WSGI app object must be set elsewhere.
>
>         i.e. to instantiate WSGIServer and set it up, you need to call
>
>           s = wsgiref.simple_server.WSGIServer((host, port),
>                                    wsgiref.simple_server.RequestHandler)
>           s.set_app(app)
>
>         This seems *very* counterintuitive and overcomplex to me, and
>         it seems to be entirely for the benefit of complying with an
>         interface inherited from code that WSGI newbies probably won't
>         be using anyway.
>
>         My suggestion is to eliminate this complexity and simply make it
>
>           s = wsgiref.WSGIServer(host, port, app)
>
>         or (if you still insist on the performance enhancement of
>         keeping it under a separate module ;)
>
>           s = wsgiref.simple_server.WSGIServer(host, port, app)
>
>         What do you think?

I think that to avoid breaking compatibility with existing programs that 
use WSGIServer I could see supporting WSGIServer((host,port),app), (with a 
default app of None) but not (host, port, app) unless another constructor 
was added.


>   * the get_app/set_app functions must be due to support for python
>         2.1; they're obviously unnecessary now, what with properties in 2.2
>         and up.

Actually, I don't remember now why I did that.  I'm not even sure that 
simple_server supports Python 2.1, although most of the other wsgiref 
modules are intended to support it.


>   Should they be changed to properties, for inclusion
>         in the stdlib?

I don't see a benefit to doing so.  The internal interface between the 
handler and server objects is such that get_app() is used now, and of 
course existing code using wsgiref uses set_app().  New code is likely to 
use the constructor instead, so I don't see any benefit coming from adding 
properties.


>   * the simple_server code is completely untested by the unit tests,
>         it seems.  Do you have any thoughts on how to instrument it to
>         be tested?

Well, for unit testing purposes it seems to me that the request handler can 
be tested without having an actual connection.  Similarly, you can unit 
test the server's setup_environ() and other methods added.  (I'm assuming 
here that "unit" testing would mean testing that modicum of functionality 
which wsgiref adds to the BaseHTTP stuff.)


>         (The easiest way for me to test it would be to install
>         wsgi_intercept and use urllib2 to run various mock HTTP tests.

That sounds more like you want integration testing; I don't really see a 
way to do that.


>         I doubt people want wsgi_intercept in the stdlib, tho, even
>         if it's only used for tests...)

Why not?  Is there something wrong with it?  :)


>   * finally, there are a lot of blank lines loitering about in your
>         python files.  Some of them are for code spacing, some of them
>         are just hanging out at the bottom of files like
>         simple_server.py.  Do you want to leave these in?

I'd like to, but Guido wouldn't.  :)  I'll let them die a natural death 
once the code is in the stdlib.  (All of them, including the trailing ones, 
are to space out code to nicely paged screenfuls in my text editor, so I 
can navigate easily.)