lazr.restful

Merge lp:~leonardr/lazr.restful/optional-compression into lp:lazr.restful

  1. optional-compression
  2. Merge into trunk
Proposed by Leonard Richardson
Status: Merged
Merged at revision: not available
Proposed branch: lp:~leonardr/lazr.restful/optional-compression
Merge into: lp:lazr.restful
Diff against target: 163 lines
7 files modified
src/lazr/restful/NEWS.txt (+7/-0)
src/lazr/restful/_resource.py (+6/-0)
src/lazr/restful/example/base/tests/root.txt (+21/-0)
src/lazr/restful/example/wsgi/root.py (+3/-3)
src/lazr/restful/interfaces/_rest.py (+14/-0)
src/lazr/restful/version.txt (+1/-1)
src/lazr/restful/wsgi.py (+12/-2)
To merge this branch: bzr merge lp:~leonardr/lazr.restful/optional-compression
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Review via email: mp+13358@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

This branch fixes bug 450570; see there for details. Basically I create a configuration option that will stop lazr.restful from setting any hop-to-hop headers, and create a BaseWSGIConfiguration that sets that configuration value to false. This will prevent lazr.restful from doing things like setting the Transfer-Encoding header, which greatly displeases WSGI servers. (It doesn't displease the WSGI server I use in my tests, which is why this behavior is not tested directly, but it does displease real servers.)

It's a little unusual to change the web service configuration from a pagetest, but I thought root.txt was the best place for the new test since that's where existing compression behavior is tested.

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Leonard,

This branch looks good. I just have two comments below.

merge-approved

-Edwin

>=== modified file 'src/lazr/restful/interfaces/_rest.py'
>--- src/lazr/restful/interfaces/_rest.py 2009-09-08 12:40:55 +0000
>+++ src/lazr/restful/interfaces/_rest.py 2009-10-14 18:14:57 +0000
>@@ -454,6 +454,20 @@
> "a collection, they will not be allowed to request more entries "
> "in the batch than this.")
>
>+ set_hop_to_hop_headers = Bool(
>+ title=u"Whether your application server will allow lazr.restful "
>+ "to set hop-top-hop headers.",

s/-top-/-to-/
I had never heard of hop-to-hop before, so I looked it up. It
seems that it is normally referred to as hop-by-hop. It is reasonably
clear as-is, so this is not a requirement, just a suggestion.

>+ default=True,
>+ description=u"Ordinarily, lazr.restful automatically compresses "
>+ "outgoing representations and sets the Transfer-Encoding "
>+ "header accordingly. (Apache's mod_compress does something "
>+ "similar, but it sets the Content-Encoding header, which means "
>+ "it has to munge the ETag header, which is no good for our purposes.) "
>+ "But WSGI application servers prohibit intermediaries from "
>+ "setting hop-to-hop headers like Transfer-Encoding. The best you can "
>+ "do in that situation is avoid setting those headers and use a "
>+ "web server that does compression using Transfer-Encoding.")
>+
> def createRequest(body_instream, environ):
> """A factory method that creates a request for the web service.
>
>
>=== modified file 'src/lazr/restful/wsgi.py'
>--- src/lazr/restful/wsgi.py 2009-10-07 12:53:04 +0000
>+++ src/lazr/restful/wsgi.py 2009-10-14 18:14:57 +0000
>@@ -2,6 +2,7 @@
>
> __metaclass__ = type
> __all__ = [
>+ 'BaseWSGIWebServiceConfiguration',
> 'WSGIApplication',
> ]
>
>@@ -15,7 +16,8 @@

pyflakes says this file has an unused import.
    from zope.interface import implements

>
> from lazr.restful.interfaces import (
> IWebServiceConfiguration, IServiceRootResource)
>-from lazr.restful.simple import Publication, Request
>+from lazr.restful.simple import (
>+ BaseWebServiceConfiguration, Publication, Request)
>
>
> class WSGIApplication:
>@@ -60,3 +62,12 @@
> """Create a WSGI server object for a particular web service."""
> cls.configure_server(host, port, config_package, config_file)
> return wsgi_make_server(host, int(port), cls)
>+
>+
>+class BaseWSGIWebServiceConfiguration(BaseWebServiceConfiguration):
>+ """A basic web service configuration optimized for WSGI apps.
>+
>+ The only difference between this and the default
>+ WebServiceConfiguration is that set_hop_to_hop_headers is False.
>+ """
>+ set_hop_to_hop_headers = False

review: Approve (code)
lp:~leonardr/lazr.restful/optional-compression updated
85. By Leonard Richardson

Response to feedback.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt 2009-10-12 20:47:21 +0000
+++ src/lazr/restful/NEWS.txt 2009-10-14 20:02:10 +0000
@@ -2,6 +2,13 @@
2NEWS for lazr.restful2NEWS for lazr.restful
3=====================3=====================
44
50.9.12 (2009-10-14)
6===================
7
8Transparent compression using Transfer-Encoding is now optional and
9disabled by default for WSGI applications. (Real WSGI servers don't
10allow applications to set hop-by-hop headers like Transfer-Encoding.)
11
50.9.11 (2009-10-12)120.9.11 (2009-10-12)
6===================13===================
714
815
=== modified file 'src/lazr/restful/_resource.py'
--- src/lazr/restful/_resource.py 2009-10-07 17:12:24 +0000
+++ src/lazr/restful/_resource.py 2009-10-14 20:02:10 +0000
@@ -342,7 +342,13 @@
342 This method has side effects. If an encoding was applied, it342 This method has side effects. If an encoding was applied, it
343 sets the "Transfer-Encoding" response header to the name of343 sets the "Transfer-Encoding" response header to the name of
344 that encoding.344 that encoding.
345
346 If the web service configuration prohibits the use of
347 hop-by-hop headers, this method does nothing.
345 """348 """
349 if not getUtility(IWebServiceConfiguration).set_hop_by_hop_headers:
350 return representation
351
346 if representation == "":352 if representation == "":
347 # Don't compress an empty representation--that makes it nonempty.353 # Don't compress an empty representation--that makes it nonempty.
348 return ""354 return ""
349355
=== modified file 'src/lazr/restful/example/base/tests/root.txt'
--- src/lazr/restful/example/base/tests/root.txt 2009-07-08 14:09:00 +0000
+++ src/lazr/restful/example/base/tests/root.txt 2009-10-14 20:02:10 +0000
@@ -144,6 +144,27 @@
144 >>> response.getheader("Transfer-Encoding")144 >>> response.getheader("Transfer-Encoding")
145 'gzip'145 'gzip'
146146
147If the configuration variable set_hop_by_hop_headers is false,
148lazr.restful will never compress representations, because it's not
149allowed to set the Transfer-Encoding header.
150
151 >>> from zope.component import getUtility
152 >>> from lazr.restful.interfaces import IWebServiceConfiguration
153 >>> config = getUtility(IWebServiceConfiguration)
154 >>> config.set_hop_by_hop_headers
155 True
156 >>> config.set_hop_by_hop_headers = False
157
158 >>> response = webservice.get('/', headers={'TE' : 'gzip'})
159 >>> print response.getheader("Transfer-Encoding")
160 None
161 >>> print response.body
162 {...}
163
164Restore the original value.
165
166 >>> config.set_hop_by_hop_headers = True
167
147=====================168=====================
148Top-level entry links169Top-level entry links
149=====================170=====================
150171
=== modified file 'src/lazr/restful/example/wsgi/root.py'
--- src/lazr/restful/example/wsgi/root.py 2009-09-03 14:04:03 +0000
+++ src/lazr/restful/example/wsgi/root.py 2009-10-14 20:02:10 +0000
@@ -12,8 +12,8 @@
1212
13from lazr.restful.example.wsgi.resources import (13from lazr.restful.example.wsgi.resources import (
14 IKeyValuePair, PairSet, KeyValuePair)14 IKeyValuePair, PairSet, KeyValuePair)
15from lazr.restful.simple import (15from lazr.restful.wsgi import BaseWSGIWebServiceConfiguration
16 BaseWebServiceConfiguration, RootResource, RootResourceAbsoluteURL)16from lazr.restful.simple import RootResource, RootResourceAbsoluteURL
1717
1818
19class RootAbsoluteURL(RootResourceAbsoluteURL):19class RootAbsoluteURL(RootResourceAbsoluteURL):
@@ -32,7 +32,7 @@
32 """32 """
3333
3434
35class WebServiceConfiguration(BaseWebServiceConfiguration):35class WebServiceConfiguration(BaseWSGIWebServiceConfiguration):
36 code_revision = '1'36 code_revision = '1'
37 service_version_uri_prefix = '1.0'37 service_version_uri_prefix = '1.0'
38 use_https = False38 use_https = False
3939
=== modified file 'src/lazr/restful/interfaces/_rest.py'
--- src/lazr/restful/interfaces/_rest.py 2009-09-08 12:40:55 +0000
+++ src/lazr/restful/interfaces/_rest.py 2009-10-14 20:02:10 +0000
@@ -454,6 +454,20 @@
454 "a collection, they will not be allowed to request more entries "454 "a collection, they will not be allowed to request more entries "
455 "in the batch than this.")455 "in the batch than this.")
456456
457 set_hop_by_hop_headers = Bool(
458 title=u"Whether your application server will allow lazr.restful "
459 "to set hop-by-hop headers.",
460 default=True,
461 description=u"Ordinarily, lazr.restful automatically compresses "
462 "outgoing representations and sets the Transfer-Encoding "
463 "header accordingly. (Apache's mod_compress does something "
464 "similar, but it sets the Content-Encoding header, which means "
465 "it has to munge the ETag header, which is no good for our purposes.) "
466 "But WSGI application servers prohibit intermediaries from "
467 "setting hop-by-hop headers like Transfer-Encoding. The best you can "
468 "do in that situation is avoid setting those headers and use a "
469 "web server that does compression using Transfer-Encoding.")
470
457 def createRequest(body_instream, environ):471 def createRequest(body_instream, environ):
458 """A factory method that creates a request for the web service.472 """A factory method that creates a request for the web service.
459473
460474
=== modified file 'src/lazr/restful/version.txt'
--- src/lazr/restful/version.txt 2009-10-12 20:47:21 +0000
+++ src/lazr/restful/version.txt 2009-10-14 20:02:10 +0000
@@ -1,1 +1,1 @@
10.9.1110.9.12
22
=== modified file 'src/lazr/restful/wsgi.py'
--- src/lazr/restful/wsgi.py 2009-10-07 12:53:04 +0000
+++ src/lazr/restful/wsgi.py 2009-10-14 20:02:10 +0000
@@ -2,6 +2,7 @@
22
3__metaclass__ = type3__metaclass__ = type
4__all__ = [4__all__ = [
5 'BaseWSGIWebServiceConfiguration',
5 'WSGIApplication',6 'WSGIApplication',
6 ]7 ]
78
@@ -10,12 +11,12 @@
1011
11from zope.component import getUtility12from zope.component import getUtility
12from zope.configuration import xmlconfig13from zope.configuration import xmlconfig
13from zope.interface import implements
14from zope.publisher.publish import publish14from zope.publisher.publish import publish
1515
16from lazr.restful.interfaces import (16from lazr.restful.interfaces import (
17 IWebServiceConfiguration, IServiceRootResource)17 IWebServiceConfiguration, IServiceRootResource)
18from lazr.restful.simple import Publication, Request18from lazr.restful.simple import (
19 BaseWebServiceConfiguration, Publication, Request)
1920
2021
21class WSGIApplication:22class WSGIApplication:
@@ -60,3 +61,12 @@
60 """Create a WSGI server object for a particular web service."""61 """Create a WSGI server object for a particular web service."""
61 cls.configure_server(host, port, config_package, config_file)62 cls.configure_server(host, port, config_package, config_file)
62 return wsgi_make_server(host, int(port), cls)63 return wsgi_make_server(host, int(port), cls)
64
65
66class BaseWSGIWebServiceConfiguration(BaseWebServiceConfiguration):
67 """A basic web service configuration optimized for WSGI apps.
68
69 The only difference between this and the default
70 WebServiceConfiguration is that set_hop_by_hop_headers is False.
71 """
72 set_hop_by_hop_headers = False

Subscribers

People subscribed via source and target branches