Merge lp:~xaav/loggerhead/export-tarball into lp:loggerhead
- export-tarball
- Merge into trunk-rich
Status: | Superseded |
---|---|
Proposed branch: | lp:~xaav/loggerhead/export-tarball |
Merge into: | lp:loggerhead |
Diff against target: |
373 lines (+161/-13) (has conflicts) 9 files modified
loggerhead/apps/branch.py (+13/-2) loggerhead/config.py (+3/-0) loggerhead/controllers/__init__.py (+1/-1) loggerhead/controllers/download_ui.py (+39/-8) loggerhead/controllers/revision_ui.py (+19/-0) loggerhead/exporter.py (+46/-0) loggerhead/history.py (+5/-1) loggerhead/templates/revision.pt (+4/-1) loggerhead/tests/test_controllers.py (+31/-0) Text conflict in loggerhead/apps/branch.py Text conflict in loggerhead/controllers/revision_ui.py Text conflict in loggerhead/tests/test_controllers.py |
To merge this branch: | bzr merge lp:~xaav/loggerhead/export-tarball |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Vincent Ladeuil (community) | Needs Fixing | ||
Gavin Panella (community) | Needs Fixing | ||
Launchpad code reviewers | code | Pending | |
Robert Collins | Pending | ||
Martin Albisetti | Pending | ||
Review via email: mp+63931@code.launchpad.net |
This proposal supersedes a proposal from 2011-05-31.
This proposal has been superseded by a proposal from 2011-06-30.
Commit message
Description of the change
This branch **may** accomplish exporting the tarball using chunked transfer encoding. The code all looks to be correct, but I have not tested it, so I would like your opinion.
Thanks!
Martin Pool (mbp) wrote : Posted in a previous version of this proposal | # |
Robert Collins (lifeless) wrote : Posted in a previous version of this proposal | # |
This looks very interesting.
Three isues:
1)
+class TarExporterFile
+
+ def __init__(self):
+ self._buffer = ''
+
+ def write(self, str):
+ self._buffer += str
+
+ def get_buffer(self):
+ buffer = self._buffer
+ self._buffer = ''
+ return buffer
This is going to be somewhat inefficient. Try this:
+class TarExporterFile
+
+ def __init__(self):
+ self._buffer = []
+
+ def write(self, str):
+ self._buffer.
+
+ def get_buffer(self):
+ try:
+ return ''.join(
+ finally:
+ self._buffer = []
2) There are no tests for this. the test suite is still pretty new, but its a good idea to test things - in particular in cases like this we need to be fairly confident it will be incremental and not block on the export - I can imagine the wsgi layer buffering everything, for instance. [in fact, I'll lay odds it will unless we fix a few things].
3) The export function is a copy-paste-tweak of the core from bzrlib. This will lead to bugs as that code base evolves - we should instead get a supported function in bzrlib lib that we can import and use.
I'm putting this back to WIP - but its a great start. Please keep at it and just shout if you need pointers.
xaav (xaav) wrote : Posted in a previous version of this proposal | # |
> 3) The export function is a copy-paste-tweak of the core from bzrlib. This will lead to bugs as
> that code base evolves - we should instead get a supported function in bzrlib lib that we can
> import and use.
Well, there is only one problem with that. According to the WSGI spec, you must return an iterable that will export the blocks. If I were to call the provided function, it would be impossible to break the response into pieces because the provided function would export it all at once. I know that it is a copy and paste tweak, but there is really no way I can inject the 'yield' keyword into the provided function. If you have another suggestion, I would be glad to hear it.
Issue number one I will be glad to fix.
Regarding issue number two, I have not written tests before but I will try my best.
Robert Collins (lifeless) wrote : Posted in a previous version of this proposal | # |
On Wed, Jun 1, 2011 at 9:46 AM, Geoff <email address hidden> wrote:
>> 3) The export function is a copy-paste-tweak of the core from bzrlib. This will lead to bugs as
>> that code base evolves - we should instead get a supported function in bzrlib lib that we can
>> import and use.
>
> Well, there is only one problem with that. According to the WSGI spec, you must return an iterable that will export the blocks. If I were to call the provided function, it would be impossible to break the response into pieces because the provided function would export it all at once. I know that it is a copy and paste tweak, but there is really no way I can inject the 'yield' keyword into the provided function. If you have another suggestion, I would be glad to hear it.
Extract the function in bzrlib into two parts - a generator (what you
have here) and a consumer than consumes it all triggering the writes.
Then we can reuse the generator.
xaav (xaav) wrote : Posted in a previous version of this proposal | # |
> On Wed, Jun 1, 2011 at 9:46 AM, Geoff <email address hidden> wrote:
> >> 3) The export function is a copy-paste-tweak of the core from bzrlib. This
> will lead to bugs as
> >> that code base evolves - we should instead get a supported function in
> bzrlib lib that we can
> >> import and use.
> >
> > Well, there is only one problem with that. According to the WSGI spec, you
> must return an iterable that will export the blocks. If I were to call the
> provided function, it would be impossible to break the response into pieces
> because the provided function would export it all at once. I know that it is a
> copy and paste tweak, but there is really no way I can inject the 'yield'
> keyword into the provided function. If you have another suggestion, I would be
> glad to hear it.
>
> Extract the function in bzrlib into two parts - a generator (what you
> have here) and a consumer than consumes it all triggering the writes.
>
> Then we can reuse the generator.
Okay, I see what you mean.
xaav (xaav) wrote : Posted in a previous version of this proposal | # |
See Bug #791005 for further information on this.
xaav (xaav) wrote : Posted in a previous version of this proposal | # |
Okay, I think this should work, but I haven't tested it yet.
Martin Pool (mbp) wrote : | # |
Thanks, xaav.
When you say "haven't tested" do you mean just "not written any tests", or "not even been able to run it"?
xaav (xaav) wrote : | # |
I haven't written any tests (Sorry, I'll do this ASAP.)
I also have not been able to run it because I'm lazy and it requires too much work to get loggerhead running on W1nd0w$.
xaav (xaav) wrote : | # |
Okay, I've added a simple tarfile test. However, I am not able to run the test to I would appreciate if someone would do that for me and/or try to download a tarball from their browser.
Gavin Panella (allenap) wrote : | # |
Once I'd set up a virtualenv with the right prerequisites, I got the
following error when running the test suite:
{{{
Traceback (most recent call last):
...
File ".../loggerhead
from loggerhead.
File ".../loggerhead
from loggerhead.
File ".../loggerhead
from loggerhead.exporter import export_tarball
ImportError: cannot import name export_tarball
}}}
After fixing that I got the following error from
TestDownloadTar
{{{
Traceback (most recent call last):
File "/usr/lib/
return fn(*args, **kwargs)
File "/usr/lib/
return self._get_
File ".../loggerhead
app = self.setUpLogge
File ".../loggerhead
branch_app = BranchWSGIApp(
AttributeError: 'TestDownloadTa
}}}
Obviously this needs some work.
We've been talking about taking more of a "patch pilot" approach in
Launchpad. That seems to mean that one of the core team - fwiw, I
would be happy to do it - would actively help getting this landed,
rather than just reviewing it. Would you like that, or would you
prefer to iterate on your own?
xaav (xaav) wrote : | # |
Sorry, I had been gone. I will be sure and look into this right away!
On Thu, Jun 16, 2011 at 8:56 AM, Gavin Panella
<email address hidden>wrote:
> Review: Needs Fixing
> Once I'd set up a virtualenv with the right prerequisites, I got the
> following error when running the test suite:
>
> {{{
> Traceback (most recent call last):
> ...
> File ".../loggerhead
> from loggerhead.
> File ".../loggerhead
> from loggerhead.
> DownloadTarballUI
> File ".../loggerhead
> from loggerhead.exporter import export_tarball
> ImportError: cannot import name export_tarball
> }}}
>
> After fixing that I got the following error from
> TestDownloadTar
>
> {{{
> Traceback (most recent call last):
> File "/usr/lib/
> _run_user
> return fn(*args, **kwargs)
> File "/usr/lib/
> in _run_test_method
> return self._get_
> File ".../loggerhead
> test_download_
> app = self.setUpLogge
> File ".../loggerhead
> branch_app = BranchWSGIApp(
> AttributeError: 'TestDownloadTa
> }}}
>
> Obviously this needs some work.
>
> We've been talking about taking more of a "patch pilot" approach in
> Launchpad. That seems to mean that one of the core team - fwiw, I
> would be happy to do it - would actively help getting this landed,
> rather than just reviewing it. Would you like that, or would you
> prefer to iterate on your own?
>
> --
> https:/
> You are the owner of lp:~xaav/loggerhead/export-tarball.
>
xaav (xaav) wrote : | # |
We've been talking about taking more of a "patch pilot" approach in
Launchpad. That seems to mean that one of the core team - fwiw, I
would be happy to do it - would actively help getting this landed,
rather than just reviewing it. Would you like that, or would you
prefer to iterate on your own?
That would be great! Any help would be greatly appreciated.
xaav (xaav) wrote : | # |
Okay, I've fixed some stuff (the tests are still broken), but here is what I'm getting:
Traceback (most recent call last):
File "C:\Python27\
ine 1068, in process_
self.
File "C:\Python27\
self.
File "C:\Python27\
self.handle()
File "C:\Python27\
ine 442, in handle
BaseHTTPReq
File "C:\Python27\
self.
File "C:\Python27\
ine 437, in handle_one_request
self.
File "C:\Python27\
ine 289, in wsgi_execute
for chunk in result:
File "C:\Users\
xport_archive
for _ in get_export_
File "C:\Python27\
port_generator
root = get_root_name(dest)
File "C:\Python27\
ot_name
dest = os.path.
File "C:\Python27\
return split(p)[1]
File "C:\Python27\
d, p = splitdrive(p)
File "C:\Python27\
if p[1:2] == ':':
TypeError: 'NoneType' object is not subscriptable
Any help would be appreciated.
Gavin Panella (allenap) wrote : | # |
Cool. I am busy this week, but I might get to it. If not, next week for sure.
Vincent Ladeuil (vila) wrote : | # |
Hi,
I almost got the test running with some additional fixes
(available at lp:~vila/loggerhead/export-tarball) only to run
into a bug in bzr itself (I think you should be able to fix that
one ;).
Note that your code requires bzr >= 2.4 (launchpad only runs
2.3.3 so far) so we'll need some support from the lp guys to
deploy a more recent version there.
Summary of my fixes:
- you need to create a branch (with some content even) before
being able to call
app = self.setUpLogge
So I've added a setUp method for your class to do that. You
probably want to add *more* tests to check that you get a valid
tarball with the expected content (which an empty branch
doesn't allow ;).
- you're calling get_export_
the code in bzrlib defaults to dest to set root. This raises an
interesing point: which root should be used here (i.e. what do
we want to prefix all the paths in the archive
with). <project>-<branch nick>-<revno> may be nice (but ask
others for feedback too).
- you used '.tar.gz' for the format but bzr expects either 'tgz'
OR a dest file name to deduce the format from the file
suffix. I just used 'tgz' there.
With these fixes in place we get:
=======
ERROR: bzrlib.
-------
_StringException: Text attachment: log
------------
0.622 creating repository in file://
0.624 creating branch <bzrlib.
0.631 trying to create missing lock '/tmp/testbzr-
0.631 opening working tree '/tmp/testbzr-
0.642 export version <InventoryRevis
0.649 opening working tree '/tmp/testbzr-
------------
Text attachment: traceback
------------
Traceback (most recent call last):
File "/usr/lib/
return fn(*args, **kwargs)
File "/usr/lib/
return self._get_
File "/home/
res = app.get('/tarball')
File "/usr/lib/
return self.do_
File "/usr/lib/
**req.environ)
File "/usr/lib/
- 448. By xaav <email address hidden>
-
modified bzrignore
- 449. By xaav <email address hidden>
-
Merged branch
- 450. By xaav
-
Fixed gzip bug.
- 451. By xaav <email address hidden>
-
Merged lp:loggerhead
- 452. By xaav <email address hidden>
-
Fixed extension issue
- 453. By xaav
-
Fixed buggy merging.
- 454. By xaav <email address hidden>
-
Fixed buggy merging and removed IDE files
- 455. By xaav
-
Fixed serve tarballs issue.
- 456. By xaav
-
Fixed tests.
- 457. By xaav
-
Added to UI.
- 458. By xaav
-
UI fixes.
Unmerged revisions
Preview Diff
1 | === modified file 'loggerhead/apps/branch.py' | |||
2 | --- loggerhead/apps/branch.py 2011-06-28 16:09:37 +0000 | |||
3 | +++ loggerhead/apps/branch.py 2011-06-28 18:02:56 +0000 | |||
4 | @@ -33,7 +33,7 @@ | |||
5 | 33 | from loggerhead.controllers.atom_ui import AtomUI | 33 | from loggerhead.controllers.atom_ui import AtomUI |
6 | 34 | from loggerhead.controllers.changelog_ui import ChangeLogUI | 34 | from loggerhead.controllers.changelog_ui import ChangeLogUI |
7 | 35 | from loggerhead.controllers.diff_ui import DiffUI | 35 | from loggerhead.controllers.diff_ui import DiffUI |
9 | 36 | from loggerhead.controllers.download_ui import DownloadUI | 36 | from loggerhead.controllers.download_ui import DownloadUI, DownloadTarballUI |
10 | 37 | from loggerhead.controllers.filediff_ui import FileDiffUI | 37 | from loggerhead.controllers.filediff_ui import FileDiffUI |
11 | 38 | from loggerhead.controllers.inventory_ui import InventoryUI | 38 | from loggerhead.controllers.inventory_ui import InventoryUI |
12 | 39 | from loggerhead.controllers.revision_ui import RevisionUI | 39 | from loggerhead.controllers.revision_ui import RevisionUI |
13 | @@ -49,7 +49,7 @@ | |||
14 | 49 | 49 | ||
15 | 50 | def __init__(self, branch, friendly_name=None, config={}, | 50 | def __init__(self, branch, friendly_name=None, config={}, |
16 | 51 | graph_cache=None, branch_link=None, is_root=False, | 51 | graph_cache=None, branch_link=None, is_root=False, |
18 | 52 | served_url=_DEFAULT, use_cdn=False): | 52 | served_url=_DEFAULT, use_cdn=False, export_tarballs=True): |
19 | 53 | self.branch = branch | 53 | self.branch = branch |
20 | 54 | self._config = config | 54 | self._config = config |
21 | 55 | self.friendly_name = friendly_name | 55 | self.friendly_name = friendly_name |
22 | @@ -61,6 +61,7 @@ | |||
23 | 61 | self.is_root = is_root | 61 | self.is_root = is_root |
24 | 62 | self.served_url = served_url | 62 | self.served_url = served_url |
25 | 63 | self.use_cdn = use_cdn | 63 | self.use_cdn = use_cdn |
26 | 64 | self.export_tarballs = export_tarballs | ||
27 | 64 | 65 | ||
28 | 65 | def get_history(self): | 66 | def get_history(self): |
29 | 66 | file_cache = None | 67 | file_cache = None |
30 | @@ -126,6 +127,7 @@ | |||
31 | 126 | 'revision': RevisionUI, | 127 | 'revision': RevisionUI, |
32 | 127 | 'search': SearchUI, | 128 | 'search': SearchUI, |
33 | 128 | 'view': ViewUI, | 129 | 'view': ViewUI, |
34 | 130 | 'tarball': DownloadTarballUI, | ||
35 | 129 | } | 131 | } |
36 | 130 | 132 | ||
37 | 131 | def last_updated(self): | 133 | def last_updated(self): |
38 | @@ -176,11 +178,20 @@ | |||
39 | 176 | self.branch.lock_read() | 178 | self.branch.lock_read() |
40 | 177 | try: | 179 | try: |
41 | 178 | try: | 180 | try: |
42 | 181 | <<<<<<< TREE | ||
43 | 179 | c = self.lookup_app(environ) | 182 | c = self.lookup_app(environ) |
44 | 180 | return c(environ, start_response) | 183 | return c(environ, start_response) |
45 | 184 | ======= | ||
46 | 185 | c = cls(self, self.get_history) | ||
47 | 186 | to_ret = c(environ, start_response) | ||
48 | 187 | >>>>>>> MERGE-SOURCE | ||
49 | 181 | except: | 188 | except: |
50 | 182 | environ['exc_info'] = sys.exc_info() | 189 | environ['exc_info'] = sys.exc_info() |
51 | 183 | environ['branch'] = self | 190 | environ['branch'] = self |
52 | 184 | raise | 191 | raise |
53 | 192 | if type(to_ret) == type(httpexceptions.HTTPSeeOther('/')): | ||
54 | 193 | raise to_ret | ||
55 | 194 | else: | ||
56 | 195 | return to_ret | ||
57 | 185 | finally: | 196 | finally: |
58 | 186 | self.branch.unlock() | 197 | self.branch.unlock() |
59 | 187 | 198 | ||
60 | === modified file 'loggerhead/config.py' | |||
61 | --- loggerhead/config.py 2010-05-12 14:38:05 +0000 | |||
62 | +++ loggerhead/config.py 2011-06-28 18:02:56 +0000 | |||
63 | @@ -36,6 +36,7 @@ | |||
64 | 36 | use_cdn=False, | 36 | use_cdn=False, |
65 | 37 | sql_dir=None, | 37 | sql_dir=None, |
66 | 38 | allow_writes=False, | 38 | allow_writes=False, |
67 | 39 | export_tarballs=True, | ||
68 | 39 | ) | 40 | ) |
69 | 40 | parser.add_option("--user-dirs", action="store_true", | 41 | parser.add_option("--user-dirs", action="store_true", |
70 | 41 | help="Serve user directories as ~user.") | 42 | help="Serve user directories as ~user.") |
71 | @@ -75,6 +76,8 @@ | |||
72 | 75 | help="The directory to place the SQL cache in") | 76 | help="The directory to place the SQL cache in") |
73 | 76 | parser.add_option("--allow-writes", action="store_true", | 77 | parser.add_option("--allow-writes", action="store_true", |
74 | 77 | help="Allow writing to the Bazaar server.") | 78 | help="Allow writing to the Bazaar server.") |
75 | 79 | parser.add_option("--export-tarballs", action="store_true", | ||
76 | 80 | help="Allow exporting revisions to tarballs.") | ||
77 | 78 | return parser | 81 | return parser |
78 | 79 | 82 | ||
79 | 80 | 83 | ||
80 | 81 | 84 | ||
81 | === modified file 'loggerhead/controllers/__init__.py' | |||
82 | --- loggerhead/controllers/__init__.py 2011-06-28 16:09:37 +0000 | |||
83 | +++ loggerhead/controllers/__init__.py 2011-06-28 18:02:56 +0000 | |||
84 | @@ -21,7 +21,7 @@ | |||
85 | 21 | import simplejson | 21 | import simplejson |
86 | 22 | import time | 22 | import time |
87 | 23 | 23 | ||
89 | 24 | from paste.httpexceptions import HTTPNotFound | 24 | from paste.httpexceptions import HTTPNotFound, HTTPSeeOther |
90 | 25 | from paste.request import path_info_pop, parse_querystring | 25 | from paste.request import path_info_pop, parse_querystring |
91 | 26 | 26 | ||
92 | 27 | from loggerhead import util | 27 | from loggerhead import util |
93 | 28 | 28 | ||
94 | === modified file 'loggerhead/controllers/download_ui.py' | |||
95 | --- loggerhead/controllers/download_ui.py 2010-05-05 19:03:40 +0000 | |||
96 | +++ loggerhead/controllers/download_ui.py 2011-06-28 18:02:56 +0000 | |||
97 | @@ -19,46 +19,51 @@ | |||
98 | 19 | 19 | ||
99 | 20 | import logging | 20 | import logging |
100 | 21 | import mimetypes | 21 | import mimetypes |
101 | 22 | import os | ||
102 | 22 | import urllib | 23 | import urllib |
103 | 23 | 24 | ||
104 | 24 | from paste import httpexceptions | 25 | from paste import httpexceptions |
105 | 25 | from paste.request import path_info_pop | 26 | from paste.request import path_info_pop |
106 | 26 | 27 | ||
107 | 27 | from loggerhead.controllers import TemplatedBranchView | 28 | from loggerhead.controllers import TemplatedBranchView |
108 | 29 | from loggerhead.exporter import export_archive | ||
109 | 28 | 30 | ||
110 | 29 | log = logging.getLogger("loggerhead.controllers") | 31 | log = logging.getLogger("loggerhead.controllers") |
111 | 30 | 32 | ||
112 | 31 | 33 | ||
113 | 32 | class DownloadUI (TemplatedBranchView): | 34 | class DownloadUI (TemplatedBranchView): |
114 | 33 | 35 | ||
120 | 34 | def __call__(self, environ, start_response): | 36 | def encode_filename(self, filename): |
121 | 35 | # /download/<rev_id>/<file_id>/[filename] | 37 | |
122 | 36 | 38 | return urllib.quote(filename.encode('utf-8')) | |
123 | 37 | h = self._history | 39 | |
124 | 38 | 40 | def get_args(self, environ): | |
125 | 39 | args = [] | 41 | args = [] |
126 | 40 | while True: | 42 | while True: |
127 | 41 | arg = path_info_pop(environ) | 43 | arg = path_info_pop(environ) |
128 | 42 | if arg is None: | 44 | if arg is None: |
129 | 43 | break | 45 | break |
130 | 44 | args.append(arg) | 46 | args.append(arg) |
131 | 47 | return args | ||
132 | 45 | 48 | ||
133 | 49 | def __call__(self, environ, start_response): | ||
134 | 50 | # /download/<rev_id>/<file_id>/[filename] | ||
135 | 51 | h = self._history | ||
136 | 52 | args = self.get_args(environ) | ||
137 | 46 | if len(args) < 2: | 53 | if len(args) < 2: |
138 | 47 | raise httpexceptions.HTTPMovedPermanently( | 54 | raise httpexceptions.HTTPMovedPermanently( |
139 | 48 | self._branch.absolute_url('/changes')) | 55 | self._branch.absolute_url('/changes')) |
140 | 49 | |||
141 | 50 | revid = h.fix_revid(args[0]) | 56 | revid = h.fix_revid(args[0]) |
142 | 51 | file_id = args[1] | 57 | file_id = args[1] |
143 | 52 | path, filename, content = h.get_file(file_id, revid) | 58 | path, filename, content = h.get_file(file_id, revid) |
144 | 53 | mime_type, encoding = mimetypes.guess_type(filename) | 59 | mime_type, encoding = mimetypes.guess_type(filename) |
145 | 54 | if mime_type is None: | 60 | if mime_type is None: |
146 | 55 | mime_type = 'application/octet-stream' | 61 | mime_type = 'application/octet-stream' |
147 | 56 | |||
148 | 57 | self.log.info('/download %s @ %s (%d bytes)', | 62 | self.log.info('/download %s @ %s (%d bytes)', |
149 | 58 | path, | 63 | path, |
150 | 59 | h.get_revno(revid), | 64 | h.get_revno(revid), |
151 | 60 | len(content)) | 65 | len(content)) |
153 | 61 | encoded_filename = urllib.quote(filename.encode('utf-8')) | 66 | encoded_filename = self.encode_filename(filename) |
154 | 62 | headers = [ | 67 | headers = [ |
155 | 63 | ('Content-Type', mime_type), | 68 | ('Content-Type', mime_type), |
156 | 64 | ('Content-Length', str(len(content))), | 69 | ('Content-Length', str(len(content))), |
157 | @@ -67,3 +72,29 @@ | |||
158 | 67 | ] | 72 | ] |
159 | 68 | start_response('200 OK', headers) | 73 | start_response('200 OK', headers) |
160 | 69 | return [content] | 74 | return [content] |
161 | 75 | |||
162 | 76 | |||
163 | 77 | class DownloadTarballUI(DownloadUI): | ||
164 | 78 | |||
165 | 79 | def __call__(self, environ, start_response): | ||
166 | 80 | """Stream a tarball from a bazaar branch.""" | ||
167 | 81 | # Tried to re-use code from downloadui, not very successful | ||
168 | 82 | format = "tar" | ||
169 | 83 | history = self._history | ||
170 | 84 | self.args = self.get_args(environ) | ||
171 | 85 | if len(self.args): | ||
172 | 86 | revid = history.fix_revid(self.args[0]) | ||
173 | 87 | else: | ||
174 | 88 | revid = self.get_revid() | ||
175 | 89 | if self._branch.export_tarballs: | ||
176 | 90 | root = 'branch' | ||
177 | 91 | encoded_filename = self.encode_filename(root + format) | ||
178 | 92 | headers = [ | ||
179 | 93 | ('Content-Type', 'application/octet-stream'), | ||
180 | 94 | ('Content-Disposition', | ||
181 | 95 | "attachment; filename*=utf-8''%s" % (encoded_filename,)), | ||
182 | 96 | ] | ||
183 | 97 | start_response('200 OK', headers) | ||
184 | 98 | return export_archive(history, root, revid, format) | ||
185 | 99 | else: | ||
186 | 100 | raise httpexceptions.HTTPSeeOther('/') | ||
187 | 70 | 101 | ||
188 | === modified file 'loggerhead/controllers/revision_ui.py' | |||
189 | --- loggerhead/controllers/revision_ui.py 2011-06-27 17:11:24 +0000 | |||
190 | +++ loggerhead/controllers/revision_ui.py 2011-06-28 18:02:56 +0000 | |||
191 | @@ -136,9 +136,19 @@ | |||
192 | 136 | self._branch.friendly_name, | 136 | self._branch.friendly_name, |
193 | 137 | self._branch.is_root, | 137 | self._branch.is_root, |
194 | 138 | 'changes')) | 138 | 'changes')) |
195 | 139 | <<<<<<< TREE | ||
196 | 139 | 140 | ||
197 | 140 | values.update({ | 141 | values.update({ |
198 | 141 | 'history': self._history, | 142 | 'history': self._history, |
199 | 143 | ======= | ||
200 | 144 | can_export = self._branch.export_tarballs | ||
201 | 145 | return { | ||
202 | 146 | 'branch': self._branch, | ||
203 | 147 | 'revid': revid, | ||
204 | 148 | 'change': change, | ||
205 | 149 | 'file_changes': file_changes, | ||
206 | 150 | 'diff_chunks': diff_chunks, | ||
207 | 151 | >>>>>>> MERGE-SOURCE | ||
208 | 142 | 'link_data': simplejson.dumps(link_data), | 152 | 'link_data': simplejson.dumps(link_data), |
209 | 143 | 'json_specific_path': simplejson.dumps(path), | 153 | 'json_specific_path': simplejson.dumps(path), |
210 | 144 | 'path_to_id': simplejson.dumps(path_to_id), | 154 | 'path_to_id': simplejson.dumps(path_to_id), |
211 | @@ -149,6 +159,15 @@ | |||
212 | 149 | 'filter_file_id': filter_file_id, | 159 | 'filter_file_id': filter_file_id, |
213 | 150 | 'diff_chunks': diff_chunks, | 160 | 'diff_chunks': diff_chunks, |
214 | 151 | 'query': query, | 161 | 'query': query, |
215 | 162 | <<<<<<< TREE | ||
216 | 152 | 'specific_path': path, | 163 | 'specific_path': path, |
217 | 153 | 'start_revid': start_revid, | 164 | 'start_revid': start_revid, |
218 | 154 | }) | 165 | }) |
219 | 166 | ======= | ||
220 | 167 | 'remember': remember, | ||
221 | 168 | 'compare_revid': compare_revid, | ||
222 | 169 | 'url': self._branch.context_url, | ||
223 | 170 | 'directory_breadcrumbs': directory_breadcrumbs, | ||
224 | 171 | 'can_export': can_export, | ||
225 | 172 | } | ||
226 | 173 | >>>>>>> MERGE-SOURCE | ||
227 | 155 | 174 | ||
228 | === added file 'loggerhead/exporter.py' | |||
229 | --- loggerhead/exporter.py 1970-01-01 00:00:00 +0000 | |||
230 | +++ loggerhead/exporter.py 2011-06-28 18:02:56 +0000 | |||
231 | @@ -0,0 +1,46 @@ | |||
232 | 1 | # Copyright (C) 2011 Canonical Ltd | ||
233 | 2 | # | ||
234 | 3 | # This program is free software; you can redistribute it and/or modify | ||
235 | 4 | # it under the terms of the GNU General Public License as published by | ||
236 | 5 | # the Free Software Foundation; either version 2 of the License, or | ||
237 | 6 | # (at your option) any later version. | ||
238 | 7 | # | ||
239 | 8 | # This program is distributed in the hope that it will be useful, | ||
240 | 9 | # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
241 | 10 | # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
242 | 11 | # GNU General Public License for more details. | ||
243 | 12 | # | ||
244 | 13 | """Exports an archive from a bazaar branch""" | ||
245 | 14 | |||
246 | 15 | from bzrlib.export import get_export_generator | ||
247 | 16 | |||
248 | 17 | |||
249 | 18 | class ExporterFileObject(object): | ||
250 | 19 | |||
251 | 20 | def __init__(self): | ||
252 | 21 | self._buffer = [] | ||
253 | 22 | |||
254 | 23 | def write(self, s): | ||
255 | 24 | self._buffer.append(s) | ||
256 | 25 | |||
257 | 26 | def get_buffer(self): | ||
258 | 27 | try: | ||
259 | 28 | return ''.join(self._buffer) | ||
260 | 29 | finally: | ||
261 | 30 | self._buffer = [] | ||
262 | 31 | |||
263 | 32 | |||
264 | 33 | def export_archive(history, root, revid, format=".tar.gz"): | ||
265 | 34 | """Export tree contents to an archive | ||
266 | 35 | |||
267 | 36 | :param history: Instance of history to export | ||
268 | 37 | :param root: Root location inside the archive. | ||
269 | 38 | :param revid: Revision to export | ||
270 | 39 | :param format: Format of the archive | ||
271 | 40 | """ | ||
272 | 41 | fileobj = ExporterFileObject() | ||
273 | 42 | tree = history._branch.repository.revision_tree(revid) | ||
274 | 43 | for _ in get_export_generator(tree=tree, root=root, fileobj=fileobj, format=format): | ||
275 | 44 | yield fileobj.get_buffer() | ||
276 | 45 | # Might have additonal contents written | ||
277 | 46 | yield fileobj.get_buffer() | ||
278 | 0 | 47 | ||
279 | === modified file 'loggerhead/history.py' | |||
280 | --- loggerhead/history.py 2011-03-25 13:09:10 +0000 | |||
281 | +++ loggerhead/history.py 2011-06-28 18:02:56 +0000 | |||
282 | @@ -33,6 +33,7 @@ | |||
283 | 33 | import re | 33 | import re |
284 | 34 | import textwrap | 34 | import textwrap |
285 | 35 | import threading | 35 | import threading |
286 | 36 | import tarfile | ||
287 | 36 | 37 | ||
288 | 37 | from bzrlib import tag | 38 | from bzrlib import tag |
289 | 38 | import bzrlib.branch | 39 | import bzrlib.branch |
290 | @@ -44,6 +45,7 @@ | |||
291 | 44 | from loggerhead import search | 45 | from loggerhead import search |
292 | 45 | from loggerhead import util | 46 | from loggerhead import util |
293 | 46 | from loggerhead.wholehistory import compute_whole_history_data | 47 | from loggerhead.wholehistory import compute_whole_history_data |
294 | 48 | from bzrlib.export.tar_exporter import export_tarball | ||
295 | 47 | 49 | ||
296 | 48 | 50 | ||
297 | 49 | def is_branch(folder): | 51 | def is_branch(folder): |
298 | @@ -816,4 +818,6 @@ | |||
299 | 816 | renamed=sorted(reporter.renamed, key=lambda x: x.new_filename), | 818 | renamed=sorted(reporter.renamed, key=lambda x: x.new_filename), |
300 | 817 | removed=sorted(reporter.removed, key=lambda x: x.filename), | 819 | removed=sorted(reporter.removed, key=lambda x: x.filename), |
301 | 818 | modified=sorted(reporter.modified, key=lambda x: x.filename), | 820 | modified=sorted(reporter.modified, key=lambda x: x.filename), |
303 | 819 | text_changes=sorted(reporter.text_changes, key=lambda x: x.filename)) | 821 | text_changes=sorted(reporter.text_changes, |
304 | 822 | key=lambda x: x.filename)) | ||
305 | 823 | |||
306 | 820 | 824 | ||
307 | === added directory 'loggerhead/static/downloads' | |||
308 | === modified file 'loggerhead/templates/revision.pt' | |||
309 | --- loggerhead/templates/revision.pt 2011-06-28 16:51:55 +0000 | |||
310 | +++ loggerhead/templates/revision.pt 2011-06-28 18:02:56 +0000 | |||
311 | @@ -84,7 +84,10 @@ | |||
312 | 84 | tal:attributes="href python:url(['/diff', change.revno], clear=1)">download diff</a> | 84 | tal:attributes="href python:url(['/diff', change.revno], clear=1)">download diff</a> |
313 | 85 | <a tal:condition="python:compare_revid is not None" | 85 | <a tal:condition="python:compare_revid is not None" |
314 | 86 | tal:attributes="href python:url(['/diff', change.revno, history.get_revno(compare_revid)], clear=1)">download diff</a> | 86 | tal:attributes="href python:url(['/diff', change.revno, history.get_revno(compare_revid)], clear=1)">download diff</a> |
316 | 87 | </li> | 87 | </li> |
317 | 88 | <li tal:condition="python:can_export"> | ||
318 | 89 | <a tal:attributes="href python:url(['/tarball', change.revno])">Download tarball</a> | ||
319 | 90 | </li> | ||
320 | 88 | <li id="last"><a tal:attributes="href python:url(['/changes', change.revno]); | 91 | <li id="last"><a tal:attributes="href python:url(['/changes', change.revno]); |
321 | 89 | title string:view history from revision ${change/revno}" | 92 | title string:view history from revision ${change/revno}" |
322 | 90 | tal:content="string:view history from revision ${change/revno}"></a></li> | 93 | tal:content="string:view history from revision ${change/revno}"></a></li> |
323 | 91 | 94 | ||
324 | === modified file 'loggerhead/tests/test_controllers.py' | |||
325 | --- loggerhead/tests/test_controllers.py 2011-06-28 16:06:12 +0000 | |||
326 | +++ loggerhead/tests/test_controllers.py 2011-06-28 18:02:56 +0000 | |||
327 | @@ -1,4 +1,14 @@ | |||
328 | 1 | <<<<<<< TREE | ||
329 | 1 | import simplejson | 2 | import simplejson |
330 | 3 | ======= | ||
331 | 4 | from cStringIO import StringIO | ||
332 | 5 | import logging | ||
333 | 6 | import tarfile | ||
334 | 7 | |||
335 | 8 | from paste.httpexceptions import HTTPServerError | ||
336 | 9 | |||
337 | 10 | from bzrlib import errors | ||
338 | 11 | >>>>>>> MERGE-SOURCE | ||
339 | 2 | 12 | ||
340 | 3 | from loggerhead.apps.branch import BranchWSGIApp | 13 | from loggerhead.apps.branch import BranchWSGIApp |
341 | 4 | from loggerhead.controllers.annotate_ui import AnnotateUI | 14 | from loggerhead.controllers.annotate_ui import AnnotateUI |
342 | @@ -135,6 +145,7 @@ | |||
343 | 135 | kwargs={'file_id': 'file_id'}, headers={}) | 145 | kwargs={'file_id': 'file_id'}, headers={}) |
344 | 136 | annotated = annotate_info['annotated'] | 146 | annotated = annotate_info['annotated'] |
345 | 137 | self.assertEqual(2, len(annotated)) | 147 | self.assertEqual(2, len(annotated)) |
346 | 148 | <<<<<<< TREE | ||
347 | 138 | self.assertEqual('2', annotated[1].change.revno) | 149 | self.assertEqual('2', annotated[1].change.revno) |
348 | 139 | self.assertEqual('1', annotated[2].change.revno) | 150 | self.assertEqual('1', annotated[2].change.revno) |
349 | 140 | 151 | ||
350 | @@ -204,3 +215,23 @@ | |||
351 | 204 | revlog_ui = branch_app.lookup_app(env) | 215 | revlog_ui = branch_app.lookup_app(env) |
352 | 205 | self.assertOkJsonResponse(revlog_ui, env) | 216 | self.assertOkJsonResponse(revlog_ui, env) |
353 | 206 | 217 | ||
354 | 218 | ======= | ||
355 | 219 | self.assertEqual('2', annotated[0].change.revno) | ||
356 | 220 | self.assertEqual('1', annotated[1].change.revno) | ||
357 | 221 | |||
358 | 222 | |||
359 | 223 | class TestDownloadTarballUI(BasicTests): | ||
360 | 224 | |||
361 | 225 | def setUp(self): | ||
362 | 226 | super(TestDownloadTarballUI, self).setUp() | ||
363 | 227 | self.createBranch() | ||
364 | 228 | |||
365 | 229 | def test_download_tarball(self): | ||
366 | 230 | app = self.setUpLoggerhead() | ||
367 | 231 | res = app.get('/tarball') | ||
368 | 232 | f = open('tarball', 'w') | ||
369 | 233 | f.write(res) | ||
370 | 234 | f.close() | ||
371 | 235 | self.failIf(not tarfile.is_tarfile('tarball')) | ||
372 | 236 | # Now check the content. TBC | ||
373 | 237 | >>>>>>> MERGE-SOURCE |
Thanks very much, that'd be a really useful feature to have. Thanks
also for making it optional, because probably some installations would
not want it on.
This looks broadly reasonable -- I'm not deeply familiar with
loggerhead -- but I am very curious why you apparently reimplemented
the export-to-tarball feature. I'd rather reuse the bzr code and if
necessary change it to let it be reused here.
When you say "not tested" do you mean you haven't even run it, or only
that you didn't add automatic tests?