get_record_stream(include_delta_closure=???) wrong meaning
robertc at robertcollins.net
Mon Jun 30 23:40:58 BST 2008
On Mon, 2008-06-30 at 13:15 -0500, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> While trying to figure out how the logic was working I came across this
> The docstring says:
> ~ :param include_delta_closure: If True then the closure across any
> ~ compression parents will be included (in the opaque data).
> However, when looking at the implementations and callers, they seem to
> actually mean "as_fulltexts=True".
> Specifically, 'fetch()' passes False, because it knows it only wants the
> The Knit data converters use True, as does iter_files_bytes. And when
> you look at the implementation:
> ~ if include_delta_closure:
> ~ # XXX: get_content_maps performs its own index queries;
> allow state
> ~ # to be passed in.
> ~ text_map, _ = self._get_content_maps(present_keys)
> ~ for key in present_keys:
> ~ yield FulltextContentFactory(key, parent_map[key], None,
> ~ ''.join(text_map[key]))
> ^- This does return the "delta closure" deltas in the data stream, it
> unpacks everything into fulltexts and returns that.
> Which is okay, considering it is actually what we want, and it unpacks
> everything in a proper order (such that it performs well, doesn't have
> to double extract things, etc.).
> I'm mostly just arguing that the parameter is misleading, and it should
> be something like "return_fulltexts = True" or something else like it.
It could be renamed that yes.
> This may be something that Robert was thinking in terms of a Remote api,
> where it would return the deltas, and then the local abstraction might
> turn them back into fulltexts. But it seems like it would be better to
> request what you actually are asking for.
I don't know that anyone _cares_ to get the knit hunks when they want
fulltexts; but at least in my initial concept it was optional;
implementation is lacking.
I'm happy with renaming it.
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080701/c3f6e428/attachment.pgp
More information about the bazaar