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?
No
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):
[...]
try:
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.
Gary
_______________________________________________
Zope3-dev mailing list
Zope3-dev@zope.org
Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
