Merge lp://staging/~xaav/loggerhead/export-tarball into lp://staging/loggerhead

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?

This looks very interesting.

Three isues:

1)
+class TarExporterFileObject(object):
+
+ 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 TarExporterFileObject(object):
+
+ def __init__(self):
+ self._buffer = []
+
+ def write(self, str):
+ self._buffer.append(str)
+
+ def get_buffer(self):
+ try:
+ return ''.join(self._buffer)
+ 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.

> 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.

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.

> 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.

See Bug #791005 for further information on this.

Okay, I think this should work, but I haven't tested it yet.