[Zope-dev] Re: Zope 2.8: ZODB fix breaks undoable_transactions
Hi! Tim Peters wrote: [yuppie] http://svn.zope.org/?view=revrev=30334 changed the behavior of undoInfo() in a way that is not backwards compatible. That's true, or at least off-by-one different than recent ZODB 3.2s. Rev 30334 fixed two bugs in the implementation, so that the behavior matched what the documentation has always said undoInfo() did. I don't know when the implementation got out of synch with the docs, AFAICS there were no related changes in App.Undo.UndoSupport or undo.dtml within the last 5 years, so I guess the 'broken' behavior existed for quite a while and all existing code that uses undoInfo() or undoable_transactions() depends on the old behavior. but however people want to resolve this I will not leave the implementation disagreeing with the docs. Don't know what other people think. I believe restoring the old undoInfo behavior and adjusting the documentation would be the best solution. Fixing this in undoable_transactions would fork the behavior of both methods and fixing all products that depend on the old behavior would cause unnecessary trouble. Cheers, Yuppie ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: Zope 2.8: ZODB fix breaks undoable_transactions
[yuppie]
...
Don't know what other people think. I believe restoring the old undoInfo
behavior and adjusting the documentation would be the best solution.
Fixing this in undoable_transactions would fork the behavior of both
methods and fixing all products that depend on the old behavior would
cause unnecessary trouble.
Can you document the behavior you want? Because there were multiple
bugs in the implementation before, I'm not exactly sure what old
behavior was in all cases; I'm certain that _some_ of it was purely
accidental, never intended, and utterly surprising (when last 0).
ZODB/interfaces.py's IStorageUndoable.undoLog documents the current
behavior, which matches what ZODB's UML docs have always claimed
behavior should be. This behavior is tested in ZODB too now, so any
change here requires fiddling code, docs and tests. If Zope requires
particular behaviors, it should grow tests for those too.
I'd be happy enough changing `first` and `last` to act like Python
slice indices instead, with the caveat that because there's other
weird non-Python behavior mandated when last 0 (then undo{Log,Info}
are documented as taking the absolute value of `last` as being an
upper bound on the # of results to return -- and old behavior was
related to that, albeit with bugs of its own), they cannot act like
Python slice indices unless `first` and `last` are both non-negative.
___
Zope-Dev maillist - Zope-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: Zope 2.8: ZODB fix breaks undoable_transactions
Tim Peters wrote:
[yuppie]
...
Don't know what other people think. I believe restoring the old undoInfo
behavior and adjusting the documentation would be the best solution.
Fixing this in undoable_transactions would fork the behavior of both
methods and fixing all products that depend on the old behavior would
cause unnecessary trouble.
Can you document the behavior you want? Because there were multiple
bugs in the implementation before, I'm not exactly sure what old
behavior was in all cases; I'm certain that _some_ of it was purely
accidental, never intended, and utterly surprising (when last 0).
ZODB/interfaces.py's IStorageUndoable.undoLog documents the current
behavior, which matches what ZODB's UML docs have always claimed
behavior should be. This behavior is tested in ZODB too now, so any
change here requires fiddling code, docs and tests. If Zope requires
particular behaviors, it should grow tests for those too.
I'd be happy enough changing `first` and `last` to act like Python
slice indices instead, with the caveat that because there's other
weird non-Python behavior mandated when last 0 (then undo{Log,Info}
are documented as taking the absolute value of `last` as being an
upper bound on the # of results to return -- and old behavior was
related to that, albeit with bugs of its own), they cannot act like
Python slice indices unless `first` and `last` are both non-negative.
Zope 2 uses 'undoInfo' only in one place: 'undoable_transactions'. And
'undoable_transactions' is only used for the undo tab.
CMF uses 'undoable_transactions' in a similar way than Zope 2. The
problem with the CMF code is that it should work in a consistent way
with Zope 2.7 and 2.8.
These are the two use cases I'm aware of. Both only use last 0 and
both expect slicing behavior for positive values, e.g. these conditions
should be True if we don't change undoable_transactions::
db.undoInfo(0, 20) == db.undoInfo(0, 99)[0:20]
db.undoInfo(20, 40) == db.undoInfo(0, 99)[20:40]
I don't care very much *how* this is resolved. All I want is to get the
regressions in Zope 2 and CMF fixed.
Cheers, Yuppie
___
Zope-Dev maillist - Zope-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: Zope 2.8: ZODB fix breaks undoable_transactions
[yuppie] ... These are the two use cases I'm aware of. Both only use last 0 and both expect slicing behavior for positive values, e.g. these conditions should be True if we don't change undoable_transactions:: db.undoInfo(0, 20) == db.undoInfo(0, 99)[0:20] db.undoInfo(20, 40) == db.undoInfo(0, 99)[20:40] I'm willing to change undoInfo to do that; the old UML docs will just be wrong then. I don't care very much *how* this is resolved. All I want is to get the regressions in Zope 2 and CMF fixed. If it continues to be the case that Zope contains no tests verifying the behavior(s) it relies on, I'll have no way to know whether that stuff is fixed or not. ZODB will pass its own tests, but that's not enough (the ZODB tests have been passing all along). ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
