[MERGE] Fix a case of look before you leap in knit.py
robertc at robertcollins.net
Wed Aug 22 22:39:17 BST 2007
On Wed, 2007-08-22 at 13:27 -0400, John Arbash Meinel wrote:
> John Arbash Meinel has voted tweak.
> Status is now: Conditionally approved
> def _get_node(self, version_id):
> - return
> + try:
> + return
> + except IndexError:
> + raise RevisionNotPresent(version_id, self)
> ^- I think you are actually supposed to be raising a filename here, not
> an object.
There is no filename to raise; and RevisionNotPresent just wants a
'weave' object - the format string is 'Revision %s not present in %s'.
> At the very least, your object should have a __str__ that shows what
> path it is at. Otherwise debugging things when we get a bad revision id
> is really hard.
> It would seem that KnitGraphIndex doesn't have any idea what file it is
> associated with, though. Which is a real shame, because we *have*
> encountered this sort of thing. (Especially with data corruption, etc)
Right, it doesn't. I guess I can parameterise it at construction with a
file_id, but its another piece of data that could potentially skew, and
one that will only be used for __str__, so it seems ugly to do that.
> So, in the short term, this is okay. But really KGI is a bit hard to use
> for post-mortem understanding. I think part of that is the abstraction,
> because you are overlying a 'graph_index' object, which is also a bit
Right. I want to reduce the amount of calls going through these deep
layers. For clarity the layers used by a current pack repo are:
Knit: KnitGraphIndex, _KnitData
CombinedGraphIndex: [GraphIndex ...]
during a write there is also an InMemoryGraphIndex added to the
CombinedGraphIndex and wired up to the KnitGraphIndex callback, as well
as a ContainerWriter which is hooked up to the _KnitAccess object.
> And the fact that KGI is only used in your repository branch, and not by
> any "live" code (just the test suite) makes it difficult to find. (And I
> have to track InMemoryGraphIndex back through an 'import *' which means
> I can't just simply '*' to search for the import occurance.)
Is that in the test file? I'll change that.
I'm not really clear what you want changed though.
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/20070823/4ec976b1/attachment.pgp
More information about the bazaar