Re: [ZODB-Dev] TmpStore missing loadBefore

2005-06-26 Thread Sidnei da Silva
On Sun, Jun 26, 2005 at 01:09:54PM -0300, Sidnei da Silva wrote:
| We've hit a case where loadBefore would get called on TmpStore during
| plone.org migration. 
| 
| Unfortunately TmpStore doesn't implement loadBefore. There seems to be
| no tests for this. I tried to write one and provide a fix but just
| occurred me that TmpStore tries to be a 'simpler FileStorage that
| mixes with the current storage'.
| 
| Wouldn't it be simpler to make TmpStore subclass from FileStorage and
| just delegate to the current storage where appropriate?

There's a collector issue open against ZODB 3.3, but the bug is still
present in 3.4 and trunk.

http://mail.zope.org/pipermail/zope-collector-monitor/2005-February/005166.html

-- 
Sidnei da Silva
Enfold Systems, LLC.
___
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zodb-dev


RE: [ZODB-Dev] TmpStore missing loadBefore

2005-06-26 Thread Tim Peters
[Tim Peters]
 This was fixed a few weeks [ago], in rev 30825:

 http://svn.zope.org/ZODB/branches/3.4/?rev=30825view=rev

[Sidnei da Silva]
 Ah, there we go. So that was fixed after 3.4.0 was tagged, and Zope 2.8
 branch still points to the 3.4.0 tag.

All correct!

 Guess we need a new tag.

I'll make an internal 3.4.1a1 release and stitch that into Zope 2.8 branch
and Zope trunk tomorrow.  Internal means I skip most of the steps in a
full-blown release (will not create a ZODB tarball or Windows installers,
will not register it with PyPI, will not send a release announcement, ...).
I won't make any serious move toward releasing 3.4.1 before I detect serious
work elsewhere toward releasing Zope 2.8.1 or Zope 2.9 (ZODB releases are
driven by Zope releases).

 If you don't care too much about the new test, it's just a matter of
 adding the string loadBefore to this list in TmpStore.__init__:

 for method in (
 'getName', 'new_oid', 'modifiedInVersion', 'getSize',
 'undoLog', 'versionEmpty', 'sortKey', 'loadBefore',
 ):
 setattr(self, method, getattr(storage, method))

 Good, I had came to the same result with test and all *wink*. Wish I had
 looked at checkins before.

Indeed, and you're even one of the few people subscribed to zodb-checkins
wink.  BTW, Jim  I conspired last Wednesday to stop the flow of spam
coming from that list, so if anyone unsubscribed in a fit of Spam Hatred,
feel free to sign up again.

 Wouldn't it be simpler to make TmpStore subclass from FileStorage

 No, because TmpStore is used by Connection (to support savepoints), and
 Connection can't make assumptions about the _kind_ of storage in use.

 Humm... I don't see where the assumption would be except for a import
 dependency.

For example, you do a savepoint (or subtransaction) when connected to ZEO,
and so are using ClientStorage (rather than FileStorage directly).  TmpStore
only knows about a handful of things, and, e.g., knows nothing about how to
do a loadBefore().  Passing it off to FileStorage's implementation of
loadBefore() would be plain wrong in this case, because FileStorage has no
idea how to talk to ZEO either.  It has to get passed off to
ClientStorage.loadBefore().  Likewise for any storage of type S where S !=
FileStorage -- TmpStore is just a layer to manage Connection savepoints,
which are done in a way independent of the underlying storage type.

 My fear is that TmpStore does basically what FileStorage does

Ah, maybe that's where we're getting off track.  Not really; TmpStore and
FileStorage both write pickles to files, and both keep a kind of index
structure, but those are shallow similarities.  TmpStore is about 60 lines
of code total, while FileStorage is almost 2100 in FileStorage.py alone (not
counting support modules only FileStorage uses); because TmpStore _must_
defer to the underlying storage for most operations, it couldn't even
usefully take more from FileStorage.

 but get way less attention and ends up bitrotting. This is not the first
 time I've hit a (shallow) bug in TmpStore that would just be triggered by
 a fairly big or long subtransaction/savepoint.

More tests would be appreciated!  Since nobody looks at most of this code
most of the time, tests are the only reliable defense we have.  Jim
confessed last week that he deliberately didn't write TmpStore tests
originally because TmpStore is an internal implementation detail.  You'll
note that the new test I added didn't mention TmpStore either, except in
comments.  For that matter, the new test didn't use FileStorage either (it
used MinimalMemoryStorage, which is a bare-bones storage used only in the
test suite).  Tests specifically against TmpStore would be welcome.

 Anyway, you're probably in better position to judge this than I.

Possibly, but you're in a better position to judge that wink.

___
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zodb-dev