On Jul 28, 2006, at 5:05 AM, Dominik Huber wrote:

Gary Poster wrote:
I object. :-)
Thanks for the feedback!

:-) Thanks for the really clear discussion below. I think I have a better idea about what is going on.

BTW, I'm trying to squeeze a reply in before I (might) become unavailable for a few days. I should be back online no later than next Tuesday.

First, your change effectively adds _database_name to an unspoken interface for persistent IKeyReferences. That's not a good idea.
Yup, I did not respect that point.
Second, the error you are getting is an error condition and should be raised as one. Maybe a more helpful error would be an improvement. But your __cmp__ is somehow comparing against a persistent keyreference with an object that appears to have not yet been added to a connection (_p_jar)--something that should not have been able to happen with the default persistent adapter to keyreference.
Yes, that was my opinion too. But the error happen anyhow...
Are you working with a custom keyreference adaptor anywhere?
In any case, shuffling over this problem as your change does would likely cause incorrect sorting/indexing in the intids btree, which would be very bad. This is an error, and should be dealt with as one.
I tried this approach before. But how can I remove an incorrect sorting/indexing respecting the dict api without invoking the compare function?

class IntIds(Persistent, Contained):
   def register(self, ob):
           if key in self.ids: # <- causing the comparision error
                return self.ids[key]
      except SpecialCmpError:
                # clean up incorrect sorting/indexing
uid = self.ids[key] # <- causing the comparision error too
                del self.refs[uid]
                del self.ids[key]

Well, first of all, I suspect your situation, based on what I've seen so far, looks something like this:

- code creates obj P1
- code puts P1 in intids, with connection.
- code does something incorrect (either your code or code that you are relying on), probably involving a savepoint because of your discussion below, that keeps keyreference to P1 in intids BTree, but rolls back addition of P1 to connection. Right now we have the bad data (a key reference to an object without a connection in the BTree), but we don't know about it yet.
- Now code creates obj P2.
- code tries to put P2 in intids. If the BTree logic happens to make it compare against the bad keyreference to P1, you get the __cmp__ error. This is a red herring for debugging: the problem has already occurred (see above).

I'm pretty sure your BTree is already bad by the time you get to the failing __cmp__. The only way I know to fix it at this late stage in the story is drastic: you'll need to create a new BTree, iterate over the old one and add all the contents except for the broken keyreferences, and replace the old intid BTree with the new one.

I don't think that's appropriate error catching for the BTree code. I think if you get the error you describe, you have a serious problem in "your" code (i.e., code that you've written or code that you're relying on in your stack, like Zope). How--and when--to handle it is application policy, not something that should be in the base intids, I think.

The error occurs after a savepoint.rollback. I can reproduce the error within the basic error story (-> see testcase). It happens if we compare a commited persistent object with a rollbacked persistent object.

The current implementation hurts the demeter's law. A fresh instance does not support the db attribute, even though the situation showed within the basic error story

   >>> from persistent import Persistent
   >>> p = Persistent()
   >>> p._p_jar.db().database_name
   Traceback (most recent call last):
   AttributeError: 'NoneType' object has no attribute 'db'

I remember knowing Demeter's law. ;-) I should look it up, but I've already spent more time than I should have this morning. The above behavior is expected. The connection.add(obj) code in the persistent keyreference code gives the obj a _p_jar.

IMO the error should still be handled by KeyReferenceToPersistent __cmp__ method.

This is too late. Anything at this stage in the game is going to be a dangerous cover-up of an already serious problem.

HTH; I'll reply again next week if others haven't piped up.

Zope3-dev mailing list
Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com

Reply via email to