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
1=== modified file 'src/lazr/restful/NEWS.txt'
2--- src/lazr/restful/NEWS.txt 2009-10-12 20:47:21 +0000
3+++ src/lazr/restful/NEWS.txt 2009-10-14 20:02:10 +0000
4@@ -2,6 +2,13 @@
5 NEWS for lazr.restful
6 =====================
7
8+0.9.12 (2009-10-14)
9+===================
10+
11+Transparent compression using Transfer-Encoding is now optional and
12+disabled by default for WSGI applications. (Real WSGI servers don't
13+allow applications to set hop-by-hop headers like Transfer-Encoding.)
14+
15 0.9.11 (2009-10-12)
16 ===================
17
18
19=== modified file 'src/lazr/restful/_resource.py'
20--- src/lazr/restful/_resource.py 2009-10-07 17:12:24 +0000
21+++ src/lazr/restful/_resource.py 2009-10-14 20:02:10 +0000
22@@ -342,7 +342,13 @@
23 This method has side effects. If an encoding was applied, it
24 sets the "Transfer-Encoding" response header to the name of
25 that encoding.
26+
27+ If the web service configuration prohibits the use of
28+ hop-by-hop headers, this method does nothing.
29 """
30+ if not getUtility(IWebServiceConfiguration).set_hop_by_hop_headers:
31+ return representation
32+
33 if representation == "":
34 # Don't compress an empty representation--that makes it nonempty.
35 return ""
36
37=== modified file 'src/lazr/restful/example/base/tests/root.txt'
38--- src/lazr/restful/example/base/tests/root.txt 2009-07-08 14:09:00 +0000
39+++ src/lazr/restful/example/base/tests/root.txt 2009-10-14 20:02:10 +0000
40@@ -144,6 +144,27 @@
41 >>> response.getheader("Transfer-Encoding")
42 'gzip'
43
44+If the configuration variable set_hop_by_hop_headers is false,
45+lazr.restful will never compress representations, because it's not
46+allowed to set the Transfer-Encoding header.
47+
48+ >>> from zope.component import getUtility
49+ >>> from lazr.restful.interfaces import IWebServiceConfiguration
50+ >>> config = getUtility(IWebServiceConfiguration)
51+ >>> config.set_hop_by_hop_headers
52+ True
53+ >>> config.set_hop_by_hop_headers = False
54+
55+ >>> response = webservice.get('/', headers={'TE' : 'gzip'})
56+ >>> print response.getheader("Transfer-Encoding")
57+ None
58+ >>> print response.body
59+ {...}
60+
61+Restore the original value.
62+
63+ >>> config.set_hop_by_hop_headers = True
64+
65 =====================
66 Top-level entry links
67 =====================
68
69=== modified file 'src/lazr/restful/example/wsgi/root.py'
70--- src/lazr/restful/example/wsgi/root.py 2009-09-03 14:04:03 +0000
71+++ src/lazr/restful/example/wsgi/root.py 2009-10-14 20:02:10 +0000
72@@ -12,8 +12,8 @@
73
74 from lazr.restful.example.wsgi.resources import (
75 IKeyValuePair, PairSet, KeyValuePair)
76-from lazr.restful.simple import (
77- BaseWebServiceConfiguration, RootResource, RootResourceAbsoluteURL)
78+from lazr.restful.wsgi import BaseWSGIWebServiceConfiguration
79+from lazr.restful.simple import RootResource, RootResourceAbsoluteURL
80
81
82 class RootAbsoluteURL(RootResourceAbsoluteURL):
83@@ -32,7 +32,7 @@
84 """
85
86
87-class WebServiceConfiguration(BaseWebServiceConfiguration):
88+class WebServiceConfiguration(BaseWSGIWebServiceConfiguration):
89 code_revision = '1'
90 service_version_uri_prefix = '1.0'
91 use_https = False
92
93=== modified file 'src/lazr/restful/interfaces/_rest.py'
94--- src/lazr/restful/interfaces/_rest.py 2009-09-08 12:40:55 +0000
95+++ src/lazr/restful/interfaces/_rest.py 2009-10-14 20:02:10 +0000
96@@ -454,6 +454,20 @@
97 "a collection, they will not be allowed to request more entries "
98 "in the batch than this.")
99
100+ set_hop_by_hop_headers = Bool(
101+ title=u"Whether your application server will allow lazr.restful "
102+ "to set hop-by-hop headers.",
103+ default=True,
104+ description=u"Ordinarily, lazr.restful automatically compresses "
105+ "outgoing representations and sets the Transfer-Encoding "
106+ "header accordingly. (Apache's mod_compress does something "
107+ "similar, but it sets the Content-Encoding header, which means "
108+ "it has to munge the ETag header, which is no good for our purposes.) "
109+ "But WSGI application servers prohibit intermediaries from "
110+ "setting hop-by-hop headers like Transfer-Encoding. The best you can "
111+ "do in that situation is avoid setting those headers and use a "
112+ "web server that does compression using Transfer-Encoding.")
113+
114 def createRequest(body_instream, environ):
115 """A factory method that creates a request for the web service.
116
117
118=== modified file 'src/lazr/restful/version.txt'
119--- src/lazr/restful/version.txt 2009-10-12 20:47:21 +0000
120+++ src/lazr/restful/version.txt 2009-10-14 20:02:10 +0000
121@@ -1,1 +1,1 @@
122-0.9.11
123+0.9.12
124
125=== modified file 'src/lazr/restful/wsgi.py'
126--- src/lazr/restful/wsgi.py 2009-10-07 12:53:04 +0000
127+++ src/lazr/restful/wsgi.py 2009-10-14 20:02:10 +0000
128@@ -2,6 +2,7 @@
129
130 __metaclass__ = type
131 __all__ = [
132+ 'BaseWSGIWebServiceConfiguration',
133 'WSGIApplication',
134 ]
135
136@@ -10,12 +11,12 @@
137
138 from zope.component import getUtility
139 from zope.configuration import xmlconfig
140-from zope.interface import implements
141 from zope.publisher.publish import publish
142
143 from lazr.restful.interfaces import (
144 IWebServiceConfiguration, IServiceRootResource)
145-from lazr.restful.simple import Publication, Request
146+from lazr.restful.simple import (
147+ BaseWebServiceConfiguration, Publication, Request)
148
149
150 class WSGIApplication:
151@@ -60,3 +61,12 @@
152 """Create a WSGI server object for a particular web service."""
153 cls.configure_server(host, port, config_package, config_file)
154 return wsgi_make_server(host, int(port), cls)
155+
156+
157+class BaseWSGIWebServiceConfiguration(BaseWebServiceConfiguration):
158+ """A basic web service configuration optimized for WSGI apps.
159+
160+ The only difference between this and the default
161+ WebServiceConfiguration is that set_hop_by_hop_headers is False.
162+ """
163+ set_hop_by_hop_headers = False

Subscribers

People subscribed via source and target branches