[Web-SIG] wsgiref problems & quibbles

[Titus Brown]

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.

===

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.

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'??

Patch attached, along with 'break-wsgiref.py'.

===

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?

  * 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.  Should they be changed to properties, for inclusion
	in the stdlib?

  * 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?
	
	(The easiest way for me to test it would be to install
	wsgi_intercept and use urllib2 to run various mock HTTP tests.
	I doubt people want wsgi_intercept in the stdlib, tho, even
	if it's only used for tests...)

  * 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?

Apologies for the minor quibbles, but I'm trying to take the chance to
go over the module *before* it becomes (more or less) fixed in stone...!

cheers,
--titus
-------------- next part --------------
Index: src/wsgiref/tests/test_handlers.py
===================================================================
--- src/wsgiref/tests/test_handlers.py	(revision 2131)
+++ src/wsgiref/tests/test_handlers.py	(working copy)
@@ -170,7 +170,7 @@
 
         stdpat = (
             r"HTTP/%s 200 OK\r\n"
-            r"Date: \w{3} \w{3} [ 0123]\d \d\d:\d\d:\d\d \d{4}\r\n"
+            r"Date: \w{3}, \d\d \w{3} \d{4} \d\d:\d\d:\d\d \w{3}\r\n"
             r"%s" r"Content-Length: 0\r\n" r"\r\n"
         )
         shortpat = (
@@ -203,6 +203,7 @@
                             (stdpat%(version,sw), h.stdout.getvalue())
                         )
 
+
 TestClasses = (
     HandlerTests,
 )
Index: src/wsgiref/handlers.py
===================================================================
--- src/wsgiref/handlers.py	(revision 2131)
+++ src/wsgiref/handlers.py	(working copy)
@@ -129,7 +129,7 @@
         in the event loop to iterate over the data, and to call
         'self.close()' once the response is finished.
         """
-        if not self.result_is_file() and not self.sendfile():
+        if not self.result_is_file() or not self.sendfile():
             for data in self.result:
                 self.write(data)
             self.finish_content()
-------------- next part --------------
A non-text attachment was scrubbed...
Name: break-wsgiref.py
Type: text/x-python
Size: 643 bytes
Desc: not available
Url : http://mail.python.org/pipermail/web-sig/attachments/20060503/fe753e4d/attachment.py