loggerhead

Merge lp:~mbp/loggerhead/240580-tarball into lp:loggerhead

  1. 240580-tarball
  2. Merge into trunk-rich
Proposed by Martin Pool
Status: Merged
Approved by: Robert Collins
Approved revision: 475
Merged at revision: 461
Proposed branch: lp:~mbp/loggerhead/240580-tarball
Merge into: lp:loggerhead
Prerequisite: lp:~xaav/loggerhead/export-tarball
Diff against target: 498 lines (+209/-65)
12 files modified
info.py (+3/-2)
loggerhead/__init__.py (+1/-1)
loggerhead/apps/branch.py (+9/-10)
loggerhead/controllers/download_ui.py (+20/-13)
loggerhead/exporter.py (+18/-4)
loggerhead/history.py (+0/-1)
loggerhead/templates/branchinfo.pt (+2/-1)
loggerhead/templates/menu.pt (+0/-4)
loggerhead/templates/revision.pt (+2/-2)
loggerhead/tests/fixtures.py (+43/-0)
loggerhead/tests/test_controllers.py (+96/-14)
loggerhead/tests/test_simple.py (+15/-13)
To merge this branch: bzr merge lp:~mbp/loggerhead/240580-tarball
Reviewer Review Type Date Requested Status
Robert Collins Approve
Review via email: mp+83364@code.launchpad.net

Description of the change

This fixes up a few more things with a view to finally landing the export-tarball feature.

 - incidentally cleans up test code
 - tarballs are tgz by default
 - add tests for this
 - tarballs include the revspec (typically revno) in the name

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

i tested the scalability and memory usage of this with an 800MB ulimit, running locally, downloading a tarball of the lp tree, which ends up being 68MB. At 500MB it runs out of memory extracting the text.

Revision history for this message
Robert Collins (lifeless) wrote :

I tihnk this is fine. I'd love the shim for exporting to be in bzr itself, but that is separate to improving loggerhead today.

Revision history for this message
Robert Collins (lifeless) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'info.py'
2--- info.py 2011-03-19 08:35:57 +0000
3+++ info.py 2011-11-25 02:37:27 +0000
4@@ -3,11 +3,12 @@
5
6 bzr_plugin_name = "loggerhead"
7
8-bzr_plugin_version = (1, 18, 0)
9+bzr_plugin_version = (1, 18, 1) # Keep in sync with loggerhead/__init__.py
10
11 bzr_compatible_versions = [
12 (1, 17, 0), (1, 18, 0), (2, 0, 0), (2, 1, 0), (2, 2, 0), (2, 3, 0),
13- (2, 4, 0)]
14+ (2, 4, 0), (2, 5, 0),
15+ ]
16
17 bzr_minimum_version = bzr_compatible_versions[0]
18
19
20=== modified file 'loggerhead/__init__.py'
21--- loggerhead/__init__.py 2011-03-24 03:40:33 +0000
22+++ loggerhead/__init__.py 2011-11-25 02:37:27 +0000
23@@ -22,7 +22,7 @@
24
25 import pkg_resources
26
27-__version__ = '1.18.1'
28+__version__ = '1.18.1' # Keep in sync with ../info.py.
29 required_bzrlib = (1, 17)
30
31 pkg_resources.get_distribution('Paste>=1.6')
32
33=== modified file 'loggerhead/apps/branch.py'
34--- loggerhead/apps/branch.py 2011-11-25 02:37:27 +0000
35+++ loggerhead/apps/branch.py 2011-11-25 02:37:27 +0000
36@@ -13,7 +13,7 @@
37 # You should have received a copy of the GNU General Public License
38 # along with this program; if not, write to the Free Software
39 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
40-#
41+
42 """The WSGI application for serving a Bazaar branch."""
43
44 import logging
45@@ -50,11 +50,13 @@
46
47 def __init__(self, branch, friendly_name=None, config={},
48 graph_cache=None, branch_link=None, is_root=False,
49-<<<<<<< TREE
50- served_url=_DEFAULT, use_cdn=False, private=False):
51-=======
52- served_url=_DEFAULT, use_cdn=False, export_tarballs=True):
53->>>>>>> MERGE-SOURCE
54+ served_url=_DEFAULT, use_cdn=False, private=False,
55+ export_tarballs=True):
56+ """Create branch-publishing WSGI app.
57+
58+ :param export_tarballs: If true, allow downloading snapshots of revisions
59+ as tarballs.
60+ """
61 self.branch = branch
62 self._config = config
63 self.friendly_name = friendly_name
64@@ -66,17 +68,14 @@
65 self.is_root = is_root
66 self.served_url = served_url
67 self.use_cdn = use_cdn
68-<<<<<<< TREE
69 self.private = private
70+ self.export_tarballs = export_tarballs
71
72 def public_private_css(self):
73 if self.private:
74 return "private"
75 else:
76 return "public"
77-=======
78- self.export_tarballs = export_tarballs
79->>>>>>> MERGE-SOURCE
80
81 def get_history(self):
82 revinfo_disk_cache = None
83
84=== modified file 'loggerhead/controllers/download_ui.py'
85--- loggerhead/controllers/download_ui.py 2011-11-25 02:37:27 +0000
86+++ loggerhead/controllers/download_ui.py 2011-11-25 02:37:27 +0000
87@@ -79,22 +79,29 @@
88 def __call__(self, environ, start_response):
89 """Stream a tarball from a bazaar branch."""
90 # Tried to re-use code from downloadui, not very successful
91- format = "tar"
92+ if not self._branch.export_tarballs:
93+ raise httpexceptions.HTTPForbidden(
94+ "Tarball downloads are not allowed")
95+ archive_format = "tgz"
96 history = self._history
97 self.args = self.get_args(environ)
98 if len(self.args):
99 revid = history.fix_revid(self.args[0])
100+ version_part = '-r' + self.args[0]
101 else:
102 revid = self.get_revid()
103- if self._branch.export_tarballs:
104- root = 'branch'
105- encoded_filename = self.encode_filename(root + '.' + format)
106- headers = [
107- ('Content-Type', 'application/octet-stream'),
108- ('Content-Disposition',
109- "attachment; filename*=utf-8''%s" % (encoded_filename,)),
110- ]
111- start_response('200 OK', headers)
112- return export_archive(history, root, revid, format)
113- else:
114- raise httpexceptions.HTTPSeeOther('/')
115+ version_part = ''
116+ # XXX: Perhaps some better suggestion based on the URL or path?
117+ #
118+ # TODO: Perhaps set the tarball suggested mtime to the revision
119+ # mtime.
120+ root = self._branch.friendly_name or 'branch'
121+ encoded_filename = self.encode_filename(
122+ root + version_part + '.' + archive_format)
123+ headers = [
124+ ('Content-Type', 'application/octet-stream'),
125+ ('Content-Disposition',
126+ "attachment; filename*=utf-8''%s" % (encoded_filename,)),
127+ ]
128+ start_response('200 OK', headers)
129+ return export_archive(history, root, revid, archive_format)
130
131=== modified file 'loggerhead/exporter.py'
132--- loggerhead/exporter.py 2011-11-25 02:37:27 +0000
133+++ loggerhead/exporter.py 2011-11-25 02:37:27 +0000
134@@ -16,6 +16,16 @@
135
136
137 class ExporterFileObject(object):
138+ """Shim that accumulates temporarily written out data.
139+
140+ There are python tarfile classes that want to write to a file like object.
141+ We want to stream data. But wsgi assumes it can pull data from the
142+ handler, rather than having bytes pushed.
143+
144+ So this class holds the data temporarily, until it is pulled. It
145+ should never buffer everything because as soon as a chunk is produced,
146+ wsgi will be given the chance to take it.
147+ """
148
149 def __init__(self):
150 self._buffer = []
151@@ -29,18 +39,22 @@
152 finally:
153 self._buffer = []
154
155-
156-def export_archive(history, root, revid, format=".tar.gz"):
157+ def close(self):
158+ pass
159+
160+
161+def export_archive(history, root, revid, archive_format):
162 """Export tree contents to an archive
163
164 :param history: Instance of history to export
165 :param root: Root location inside the archive.
166 :param revid: Revision to export
167- :param format: Format of the archive
168+ :param archive_format: Format of the archive, eg 'tar.gz'.
169 """
170 fileobj = ExporterFileObject()
171 tree = history._branch.repository.revision_tree(revid)
172- for _ in get_export_generator(tree=tree, root=root, fileobj=fileobj, format=format):
173+ for _ in get_export_generator(tree=tree, root=root, fileobj=fileobj,
174+ format=archive_format):
175 yield fileobj.get_buffer()
176 # Might have additonal contents written
177 yield fileobj.get_buffer()
178
179=== modified file 'loggerhead/history.py'
180--- loggerhead/history.py 2011-11-25 02:37:27 +0000
181+++ loggerhead/history.py 2011-11-25 02:37:27 +0000
182@@ -45,7 +45,6 @@
183 from loggerhead import search
184 from loggerhead import util
185 from loggerhead.wholehistory import compute_whole_history_data
186-from bzrlib.export.tar_exporter import export_tarball
187
188
189 def is_branch(folder):
190
191=== modified file 'loggerhead/templates/branchinfo.pt'
192--- loggerhead/templates/branchinfo.pt 2009-01-23 20:53:40 +0000
193+++ loggerhead/templates/branchinfo.pt 2011-11-25 02:37:27 +0000
194@@ -1,4 +1,5 @@
195-<div id="branch-info">
196+<div id="branch-info"
197+ tal:condition="python:branch.served_url">
198 To get this branch, use: <br/>
199 <code>bzr branch
200 <tal:served-url tal:replace="python:branch.served_url" /></code>
201
202=== modified file 'loggerhead/templates/menu.pt'
203--- loggerhead/templates/menu.pt 2011-11-25 02:37:27 +0000
204+++ loggerhead/templates/menu.pt 2011-02-09 22:33:59 +0000
205@@ -6,8 +6,6 @@
206 id string:on">Changes</a></li>
207 <li><a tal:attributes="href python:url('/files', clear=1);
208 title string:Files">Files</a></li>
209- <li><a tal:attributes="href python:url(['/revision', change.revno], clear=1);
210- title string:Tarball">Tarball</a></li>
211 </tal:changes-active>
212 <tal:fileview-active tal:condition="fileview_active">
213 <li><a tal:attributes="href python:url('/changes', clear=1);
214@@ -15,8 +13,6 @@
215 <li><a tal:attributes="href python:url('/files', clear=1);
216 title string:Files;
217 id string:on">Files</a></li>
218- <li><a tal:attributes="href python:url(['/revision', change.revno], clear=1);
219- title string:Tarball">Tarball</a></li>
220 </tal:fileview-active>
221 </ul>
222 </tal:menu>
223
224=== modified file 'loggerhead/templates/revision.pt'
225--- loggerhead/templates/revision.pt 2011-11-25 02:37:27 +0000
226+++ loggerhead/templates/revision.pt 2011-11-25 02:37:27 +0000
227@@ -73,7 +73,7 @@
228 <li tal:condition="python:remember != change.revno">
229 <a tal:attributes="href python:url(['/revision', change.revno], remember=change.revno, compare_revid=None);
230 title string:compare with another revision"
231- tal:content="string:Compare with another revision"></a></li>
232+ tal:content="string:compare with another revision"></a></li>
233 <li tal:condition="python:(remember is not None) and (compare_revid is None) and (change.revno != remember)" >
234 <a tal:attributes="href python:url(['/revision', change.revno], compare_revid=history.get_revno(remember))">
235 compare with revision <tal:b content="python:history.get_revno(remember)" />
236@@ -86,7 +86,7 @@
237 tal:attributes="href python:url(['/diff', change.revno, history.get_revno(compare_revid)], clear=1)">download diff</a>
238 </li>
239 <li tal:condition="python:can_export">
240- <a tal:attributes="href python:url(['/tarball', change.revno])">Download tarball</a>
241+ <a tal:attributes="href python:url(['/tarball', change.revno])">download tarball</a>
242 </li>
243 <li id="last"><a tal:attributes="href python:url(['/changes', change.revno]);
244 title string:view history from revision ${change/revno}"
245
246=== added file 'loggerhead/tests/fixtures.py'
247--- loggerhead/tests/fixtures.py 1970-01-01 00:00:00 +0000
248+++ loggerhead/tests/fixtures.py 2011-11-25 02:37:27 +0000
249@@ -0,0 +1,43 @@
250+# Copyright (C) 2007, 2008, 2009, 2011 Canonical Ltd.
251+#
252+# This program is free software; you can redistribute it and/or modify
253+# it under the terms of the GNU General Public License as published by
254+# the Free Software Foundation; either version 2 of the License, or
255+# (at your option) any later version.
256+#
257+# This program is distributed in the hope that it will be useful,
258+# but WITHOUT ANY WARRANTY; without even the implied warranty of
259+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
260+# GNU General Public License for more details.
261+#
262+# You should have received a copy of the GNU General Public License
263+# along with this program; if not, write to the Free Software
264+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
265+
266+from __future__ import absolute_import
267+
268+from fixtures import Fixture
269+
270+
271+class SampleBranch(Fixture):
272+
273+ def __init__(self, testcase):
274+ # Must be a bzr TestCase to hook into branch creation, unfortunately.
275+ self.testcase = testcase
276+
277+ def setUp(self):
278+ Fixture.setUp(self)
279+
280+ self.tree = self.testcase.make_branch_and_tree('.')
281+
282+ self.filecontents = (
283+ 'some\nmultiline\ndata\n'
284+ 'with<htmlspecialchars\n')
285+ filenames = ['myfilename', 'anotherfile<']
286+ self.testcase.build_tree_contents(
287+ (filename, self.filecontents) for filename in filenames)
288+ for filename in filenames:
289+ self.tree.add(filename, '%s-id' % filename)
290+ self.fileid = self.tree.path2id('myfilename')
291+ self.msg = 'a very exciting commit message <'
292+ self.revid = self.tree.commit(message=self.msg)
293
294=== modified file 'loggerhead/tests/test_controllers.py'
295--- loggerhead/tests/test_controllers.py 2011-11-25 02:37:27 +0000
296+++ loggerhead/tests/test_controllers.py 2011-11-25 02:37:27 +0000
297@@ -1,19 +1,45 @@
298-<<<<<<< TREE
299-=======
300+# Copyright (C) 2008-2011 Canonical Ltd.
301+#
302+# This program is free software; you can redistribute it and/or modify
303+# it under the terms of the GNU General Public License as published by
304+# the Free Software Foundation; either version 2 of the License, or
305+# (at your option) any later version.
306+#
307+# This program is distributed in the hope that it will be useful,
308+# but WITHOUT ANY WARRANTY; without even the implied warranty of
309+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
310+# GNU General Public License for more details.
311+#
312+# You should have received a copy of the GNU General Public License
313+# along with this program; if not, write to the Free Software
314+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
315+
316 from cStringIO import StringIO
317 import logging
318 import tarfile
319+import tempfile
320
321+from paste.fixture import (
322+ AppError,
323+ )
324 from paste.httpexceptions import HTTPServerError
325
326+from testtools.matchers import (
327+ Matcher,
328+ Mismatch,
329+ )
330+
331 from bzrlib import errors
332 import simplejson
333
334->>>>>>> MERGE-SOURCE
335 from loggerhead.apps.branch import BranchWSGIApp
336 from loggerhead.controllers.annotate_ui import AnnotateUI
337 from loggerhead.controllers.inventory_ui import InventoryUI
338-from loggerhead.tests.test_simple import BasicTests, consume_app
339+from loggerhead.tests.test_simple import (
340+ BasicTests,
341+ consume_app,
342+ TestWithSimpleTree,
343+ )
344
345
346 class TestInventoryUI(BasicTests):
347@@ -242,7 +268,6 @@
348 revlog_ui = branch_app.lookup_app(env)
349 self.assertOkJsonResponse(revlog_ui, env)
350
351-<<<<<<< TREE
352
353 class TestControllerHooks(BasicTests):
354
355@@ -271,17 +296,74 @@
356 'captain hook')
357 BranchWSGIApp.hooks.install_named_hook('controller', myhook, "captain hook")
358 self.assertEquals("I am hooked", app.lookup_app(env))
359-=======
360-class TestDownloadTarballUI(BasicTests):
361+
362+
363+class IsTarfile(Matcher):
364+
365+ def __init__(self, compression):
366+ self.compression = compression
367+
368+ def match(self, content_bytes):
369+ f = tempfile.NamedTemporaryFile()
370+ try:
371+ f.write(content_bytes)
372+ f.flush()
373+ tarfile.open(f.name, mode='r|' + self.compression)
374+ finally:
375+ f.close()
376+
377+
378+class MatchesTarballHeaders(Matcher):
379+
380+ def __init__(self, expect_filename):
381+ self.expect_filename = expect_filename
382+
383+ def match(self, response):
384+ # Maybe the c-t should be more specific, but this is probably good for
385+ # making sure it gets saved without the client trying to decompress it
386+ # or anything.
387+ if (response.header('Content-Type') == 'application/octet-stream'
388+ and response.header('Content-Disposition') ==
389+ "attachment; filename*=utf-8''" + self.expect_filename):
390+ pass
391+ else:
392+ return Mismatch("wrong response headers: %r"
393+ % response.headers)
394+
395+
396+class TestDownloadTarballUI(TestWithSimpleTree):
397
398 def setUp(self):
399 super(TestDownloadTarballUI, self).setUp()
400- self.createBranch()
401
402 def test_download_tarball(self):
403- app = self.setUpLoggerhead()
404- res = app.get('/tarball')
405- f = open('tarball', 'w')
406- f.write(res)
407- f.close()
408- self.failIf(not tarfile.is_tarfile('tarball'))>>>>>>> MERGE-SOURCE
409+ # Tarball downloads are enabled by default.
410+ app = self.setUpLoggerhead()
411+ response = app.get('/tarball')
412+ self.assertThat(
413+ response.body,
414+ IsTarfile('gz'))
415+ self.assertThat(
416+ response,
417+ MatchesTarballHeaders('branch.tgz'))
418+
419+ def test_download_tarball_of_version(self):
420+ app = self.setUpLoggerhead()
421+ response = app.get('/tarball/1')
422+ self.assertThat(
423+ response.body,
424+ IsTarfile('gz'))
425+ self.assertThat(
426+ response,
427+ MatchesTarballHeaders('branch-r1.tgz'))
428+
429+ def test_download_tarball_forbidden(self):
430+ app = self.setUpLoggerhead(export_tarballs=False)
431+ e = self.assertRaises(
432+ AppError,
433+ app.get,
434+ '/tarball')
435+ self.assertContainsRe(
436+ str(e),
437+ '(?s).*403 Forbidden'
438+ '.*Tarball downloads are not allowed')
439
440=== modified file 'loggerhead/tests/test_simple.py'
441--- loggerhead/tests/test_simple.py 2011-11-25 02:37:27 +0000
442+++ loggerhead/tests/test_simple.py 2011-11-25 02:37:27 +0000
443@@ -33,6 +33,9 @@
444 from paste.fixture import TestApp
445 from paste.httpexceptions import HTTPExceptionHandler, HTTPMovedPermanently
446
447+from loggerhead.tests.fixtures import (
448+ SampleBranch,
449+ )
450
451
452 class BasicTests(TestCaseWithTransport):
453@@ -46,7 +49,7 @@
454 self.tree = self.make_branch_and_tree('.')
455
456 def setUpLoggerhead(self, **kw):
457- branch_app = BranchWSGIApp(self.tree.branch, '', export_tarballs=True, **kw).app
458+ branch_app = BranchWSGIApp(self.tree.branch, '', **kw).app
459 return TestApp(HTTPExceptionHandler(branch_app))
460
461 def assertOkJsonResponse(self, app, env):
462@@ -71,18 +74,14 @@
463
464 def setUp(self):
465 BasicTests.setUp(self)
466- self.createBranch()
467+ self.sample_branch_fixture = SampleBranch(self)
468
469- self.filecontents = ('some\nmultiline\ndata\n'
470- 'with<htmlspecialchars\n')
471- filenames = ['myfilename', 'anotherfile<']
472- self.build_tree_contents(
473- (filename, self.filecontents) for filename in filenames)
474- for filename in filenames:
475- self.tree.add(filename, '%s-id' % filename)
476- self.fileid = self.tree.path2id('myfilename')
477- self.msg = 'a very exciting commit message <'
478- self.revid = self.tree.commit(message=self.msg)
479+ # XXX: This could be cleaned up more... -- mbp 2011-11-25
480+ self.useFixture(self.sample_branch_fixture)
481+ self.tree = self.sample_branch_fixture.tree
482+ self.fileid = self.sample_branch_fixture.fileid
483+ self.filecontents = self.sample_branch_fixture.filecontents
484+ self.msg = self.sample_branch_fixture.msg
485
486 def test_public_private(self):
487 app = self.make_branch_app(self.tree.branch, private=True)
488@@ -105,7 +104,10 @@
489 res = app.get('/changes')
490 self.failUnless("To get this branch, use:" in res)
491 self.failUnless("lp:loggerhead" in res)
492- app = self.setUpLoggerhead(served_url=None)
493+
494+ def test_no_empty_download_location(self):
495+ """With no served_url, no instructions how to get it"""
496+ app = self.setUpLoggerhead()
497 res = app.get('/changes')
498 self.failIf("To get this branch, use:" in res)
499

Subscribers

People subscribed via source and target branches