[MERGE] Fix a case of look before you leap in knit.py

John Arbash Meinel john at arbash-meinel.com
Wed Aug 22 18:27:59 BST 2007


John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
      def _get_node(self, version_id):
-        return 
list(self._get_entries(self._version_ids_to_keys([version_id])))[0]
+        try:
+            return 
list(self._get_entries(self._version_ids_to_keys([version_id])))[0]
+        except IndexError:
+            raise RevisionNotPresent(version_id, self)

^- I think you are actually supposed to be raising a filename here, not 
an object.

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)

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

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

For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C1187769804.17572.104.camel%40localhost.localdomain%3E



More information about the bazaar mailing list