Merge lp://staging/~gary/lazr.restful/fix-webservice-testing-infrastructure into lp://staging/lazr.restful

Proposed by Gary Poster
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~gary/lazr.restful/fix-webservice-testing-infrastructure
Merge into: lp://staging/lazr.restful
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~gary/lazr.restful/fix-webservice-testing-infrastructure
Reviewer Review Type Date Requested Status
Leonard Richardson (community) Needs Information
Review via email: mp+10624@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

Fixes testing infrastructure and packaging problems.

Revision history for this message
Leonard Richardson (leonardr) wrote :

> Fixes testing infrastructure and packaging problems.

I have two comments.

1. Why the heck is 'roman' a dependency of lazr.restful? It's not in the code and it's such a random thing to need.

2. This comment isn't terribly helpful.

70 + # We want the initial slash of the path, so we subtract 1 from
71 + # the base_url's len.

I'd prefer something like:

# We want just one character of the base_url (the slash that begins the path), so we cut
# off the length of the base_url minus one character.

review: Needs Information
62. By Gary Poster

the missing roman dependency was because my copy of docutils had been built with a Python that had roman, and docutils tries to be clever so that meant the egg was broken for a Python without roman. removing dependency.

63. By Gary Poster

make a better comment, per review by leonardr

Revision history for this message
Gary Poster (gary) wrote :

On Aug 25, 2009, at 8:29 AM, Leonard Richardson wrote:

> Review: Needs Information
>> Fixes testing infrastructure and packaging problems.
>
> I have two comments.
>
> 1. Why the heck is 'roman' a dependency of lazr.restful? It's not in
> the code and it's such a random thing to need.

I was surprised too, but my tests failed in a clean Python (that is, a
vanilla Python with only the standard library) without it. That said,
I investigated further after you asked. It turns out that docutils
tries to do something tricky, and my version of docutils had broken
the trick. I rebuilt docutils and we're all good. Therefore, I
removed that indirect dependency from the list.

> 2. This comment isn't terribly helpful.
>
> 70 + # We want the initial slash of the path, so we subtract 1 from
> 71 + # the base_url's len.
>
> I'd prefer something like:
>
> # We want just one character of the base_url (the slash that begins
> the path), so we cut
> # off the length of the base_url minus one character.

Yes, good improvement. I took it a little further, as you can see in
the incremental diff below.

Thanks

Gary

=== modified file 'setup.py'
--- setup.py 2009-08-20 04:18:09 +0000
+++ setup.py 2009-08-25 13:46:54 +0000
@@ -61,7 +61,6 @@
          'lazr.enum',
          'lazr.lifecycle',
          'lazr.uri',
- 'roman',
          'setuptools',
          'simplejson',
          'van.testing',

=== modified file 'src/lazr/restful/testing/webservice.py'
--- src/lazr/restful/testing/webservice.py 2009-08-20 04:18:09 +0000
+++ src/lazr/restful/testing/webservice.py 2009-08-25 14:04:09 +0000
@@ -211,8 +211,9 @@
                          'preceding slash?)',
                          path_or_url)
              else:
- # We want the initial slash of the path, so we
subtract 1 from
- # the base_url's len.
+ # The base_url ends with a slash. We want that slash
to begin
+ # the path, so we cut off the length of the base_url
minus one
+ # character.
                  full_url = path_or_url[len(base_url)-1:]
          full_headers = {'Host': self.domain}
          if headers is not None:

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'MANIFEST.in'
--- MANIFEST.in 2009-03-23 19:17:56 +0000
+++ MANIFEST.in 2009-08-20 12:23:08 +0000
@@ -1,2 +1,4 @@
1include ez_setup.py
2recursive-include src *.txt *.pt *.zcml
1exclude MANIFEST.in buildout.cfg bootstrap.py .bzrignore3exclude MANIFEST.in buildout.cfg bootstrap.py .bzrignore
2prune _bootstrap4prune _bootstrap
35
=== modified file 'setup.py'
--- setup.py 2009-08-14 20:25:45 +0000
+++ setup.py 2009-08-20 04:18:09 +0000
@@ -61,6 +61,7 @@
61 'lazr.enum',61 'lazr.enum',
62 'lazr.lifecycle',62 'lazr.lifecycle',
63 'lazr.uri',63 'lazr.uri',
64 'roman',
64 'setuptools',65 'setuptools',
65 'simplejson',66 'simplejson',
66 'van.testing',67 'van.testing',
6768
=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt 2009-08-19 17:17:30 +0000
+++ src/lazr/restful/NEWS.txt 2009-08-20 04:45:22 +0000
@@ -15,6 +15,11 @@
15For services that use Django, added an adapter from Django's15For services that use Django, added an adapter from Django's
16ObjectDoesNotExist to lazr.restful's NotFoundView.16ObjectDoesNotExist to lazr.restful's NotFoundView.
1717
18Fixed some testing infrastructure in lazr.restful.testing.webservice.
19
20Added "roman" to build dependencies.
21
22Fix some critical packaging problems.
1823
190.9.4 (2009-08-17)240.9.4 (2009-08-17)
20==================25==================
2126
=== modified file 'src/lazr/restful/testing/webservice.py'
--- src/lazr/restful/testing/webservice.py 2009-08-18 15:47:10 +0000
+++ src/lazr/restful/testing/webservice.py 2009-08-20 04:18:09 +0000
@@ -8,7 +8,6 @@
8 'ExampleWebServicePublication',8 'ExampleWebServicePublication',
9 'FakeRequest',9 'FakeRequest',
10 'FakeResponse',10 'FakeResponse',
11 'MockRootFolder',
12 'pprint_entry',11 'pprint_entry',
13 'WebServiceTestPublication',12 'WebServiceTestPublication',
14 'WebServiceTestRequest',13 'WebServiceTestRequest',
@@ -183,7 +182,7 @@
183 # /firefox = http://foo.dev/firefox182 # /firefox = http://foo.dev/firefox
184 # /firefox = http://api.foo.dev/beta/firefox183 # /firefox = http://api.foo.dev/beta/firefox
185 resource_path = resource_path[1:]184 resource_path = resource_path[1:]
186 url_with_version = '/'.join(api_version, resource_path)185 url_with_version = '/'.join((api_version, resource_path))
187 return urljoin(self.base_url, url_with_version)186 return urljoin(self.base_url, url_with_version)
188187
189 def addHeadersTo(self, url, headers):188 def addHeadersTo(self, url, headers):
@@ -206,9 +205,15 @@
206 else:205 else:
207 base_url = self.base_url206 base_url = self.base_url
208 if not path_or_url.startswith(base_url):207 if not path_or_url.startswith(base_url):
209 raise ValueError('not a testing url', path_or_url)208 raise ValueError(
209 'not a testing url '
210 '(maybe you wanted a relative url but left off the '
211 'preceding slash?)',
212 path_or_url)
210 else:213 else:
211 full_url = path_or_url[len(base_url):]214 # We want the initial slash of the path, so we subtract 1 from
215 # the base_url's len.
216 full_url = path_or_url[len(base_url)-1:]
212 full_headers = {'Host': self.domain}217 full_headers = {'Host': self.domain}
213 if headers is not None:218 if headers is not None:
214 full_headers.update(headers)219 full_headers.update(headers)
@@ -354,4 +359,3 @@
354 def __str__(self):359 def __str__(self):
355 return "http://dummy"360 return "http://dummy"
356 __call__ = __str__361 __call__ = __str__
357
358362
=== modified file 'src/lazr/restful/version.txt'
--- src/lazr/restful/version.txt 2009-08-17 16:24:44 +0000
+++ src/lazr/restful/version.txt 2009-08-20 04:18:09 +0000
@@ -1,1 +1,1 @@
10.9.410.9.5

Subscribers

People subscribed via source and target branches