[Web-SIG] Re: Latest WSGI Draft (Phillip J. Eby)

[Ian Bicking]

Phillip J. Eby wrote:
>> Hmm... For a very simple filter, I actually found that surprisingly 
>> difficult to write.
> 
> 
> Maybe because you used classes unnecessarily?

It wasn't so much the result, as the process -- keeping track of the 
state was difficult for me, with one function outside of the application 
(the application wrapper) and another inside of the application (the 
start_process wrapper).  I had to remember which function fit what part 
of the process, and how to best keep the state around, and that was more 
difficult than I expected.

>     class GzipOutput(object):
>         pass
> 
>     def gzip_middleware(application, compress_level=5):
> 
>         def do_gzip(environ, start_response):
> 
>             writer = []

Using a list to simulate mutable inner scopes is hardly what I'd 
consider a Hello World class of example!  While the trick works, it's 
not something that I would do without a compelling reason; certainly not 
just to save creating one class.

>             if 'gzip' not in environ.get('HTTP_ACCEPT'):
>                 # nothing for us to do, so this middleware will
>                 # be a no-op:
>                 return application(environ, start_response)
> 
>             def gzip_start_response(status, headers):
>                 if 'content-encoding' in headers:
>                     writer.append(start_response(status,headers))
>                 else:
>                     headers['content-encoding'] = gzip
>                     raw_writer = start_response(status,headers)
>                     dummy_fileobj = GzipOutput()
>                     dummy_fileobj.write = raw_writer
>                     gzip_file = 
> GzipFile('','wb',compress_level,dummy_fileobj)
>                     writer.append(gzip_file.write)
>                 return writer[0]
> 
>             app_iter = application(environ,gzip_start_response)
> 
>             if app_iter and writer:
>                 try:
>                     map(writer[0],app_iter)
>                 finally:
>                     if hasattr(app_iter,'close'):
>                          app_iter.close()
>             else:
>                 return app_iter
> 
>         return do_gzip
> 
> Hm.  That's only slightly less complicated.  Still, the only "excise" is 
> handling the try/finally for the close -- virtually everything else is 
> directly connected to the required functionality.  (By the way, your 
> implementation tries to iterate even if the app returns None, and you 
> can't set arbitrary attributes on 'object' instances.)
> 
> It may be that the PEP should contain a list of suggested utility 
> functions, like this one:
> 
>     def finish_response(write_func,app_return):
>         if app_return:
>             try:
>                 map(write_func,app_return)
>             finally:
>                 if hasattr(app_return,'close'):
>                     app_return.close()
> 
> Such a routine would come in handy for response-munging middleware.

I believe you also have to close the GzipFile, as it won't flush its 
final output until that happens.  So the finally block has to include 
that as well.  That makes finish_response a bit less of a win.  And 
again, map is clever but something of an abuse of the function, and not 
appropriate for any example code.

>>   And I think it should take advantage of its server's iteration, but 
>> currently it only uses the "push" (write function) aspect of the 
>> server.  But I'm not sure how exactly I would do that, especially so 
>> that the iteration actually had any beneficial properties.
> 
> 
> For the given application, it's not important.  Gzipping a server push 
> stream probably doesn't make a lot of sense.  :)

How so?

-- 
Ian Bicking  /  ianb at colorstudy.com  / http://blog.ianbicking.org