Re: [ZODB-Dev] problems w/ Pickle based deep-copy of Blob objects
On Thu, 11 Sep 2008 21:20:51 +0200, Rob Miller <[EMAIL PROTECTED]> wrote: > Shane Hathaway wrote: >> Rob Miller wrote: >>> i'm assuming from the lack of response that nobody has a problem w/ >>> this >>> change... if i don't hear differently within the next 24 hours or so, >>> i'll >>> commit the change to the 3.8 branch and the trunk. >> >> The change looks harmless, but please verify it passes the ZODB tests. > > it does. > >> You should also add a test that currently fails, but passes with your >> change; that's the best way to ensure it won't break again. I recognize >> that may be fairly difficult, but there are a lot of people here who >> will answer questions. > > no problem, this one was easy: > > http://svn.zope.org/ZODB/trunk/src/ZODB/blob.py?rev=91065&view=rev i've made another fix and extended your test a bit (in http://svn.zope.org/?rev=91439&view=rev) to also allow the blob to be opened after cloning. i ran into this problem when trying to get plone.app.blob to work with versioning today, but unfortunately hadn't updated to the latest zodb yet -- see http://dev.plone.org/plone/changeset/22640. cheers, andi -- zeidler it consulting - http://zitc.de/ - [EMAIL PROTECTED] friedelstraße 31 - 12047 berlin - telefon +49 30 25563779 pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/ plone 3.1.5.1 released! -- http://plone.org/products/plone/ ___ 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] problems w/ Pickle based deep-copy of Blob objects
On Mon, 2008-09-15 at 13:19 -0700, Rob Miller wrote: > Christian Theune wrote: > > On Tue, 2008-08-19 at 12:05 -0700, Rob Miller wrote: > >> hi all, > >> > >> i've been experimenting w/ getting ZODB 3.8's blob storage support to work > >> w/ > >> the openplans.org stack, and have hit a snag about which i could use some > >> feedback. > > > > There's another bug around the corner: > > > > If CMFEditions causes new OIDs be handed out to those objects (or even > > remove them) then the blob data will disappear. > > this is because the blob file path is constructed from the OID, right? Yes. -- Christian Theune · [EMAIL PROTECTED] gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 7 · fax +49 345 1229889 1 Zope and Plone consulting and development signature.asc Description: This is a digitally signed message part ___ 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] problems w/ Pickle based deep-copy of Blob objects
Christian Theune wrote: > On Tue, 2008-08-19 at 12:05 -0700, Rob Miller wrote: >> hi all, >> >> i've been experimenting w/ getting ZODB 3.8's blob storage support to work >> w/ >> the openplans.org stack, and have hit a snag about which i could use some >> feedback. > > There's another bug around the corner: > > If CMFEditions causes new OIDs be handed out to those objects (or even > remove them) then the blob data will disappear. this is because the blob file path is constructed from the OID, right? i'll peek to see whether or not this is going to be a problem... thanks for the heads up. -r ___ 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] problems w/ Pickle based deep-copy of Blob objects
On Tue, 2008-08-19 at 12:05 -0700, Rob Miller wrote: > hi all, > > i've been experimenting w/ getting ZODB 3.8's blob storage support to work w/ > the openplans.org stack, and have hit a snag about which i could use some > feedback. There's another bug around the corner: If CMFEditions causes new OIDs be handed out to those objects (or even remove them) then the blob data will disappear. -- Christian Theune · [EMAIL PROTECTED] gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 7 · fax +49 345 1229889 1 Zope and Plone consulting and development signature.asc Description: This is a digitally signed message part ___ 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] problems w/ Pickle based deep-copy of Blob objects
Rob Miller wrote: > Shane Hathaway wrote: >> You should also add a test that currently fails, but passes with your >> change; that's the best way to ensure it won't break again. I recognize >> that may be fairly difficult, but there are a lot of people here who >> will answer questions. > > no problem, this one was easy: > > http://svn.zope.org/ZODB/trunk/src/ZODB/blob.py?rev=91065&view=rev Nice work! Shane ___ 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] problems w/ Pickle based deep-copy of Blob objects
Shane Hathaway wrote: > Rob Miller wrote: >> i'm assuming from the lack of response that nobody has a problem w/ this >> change... if i don't hear differently within the next 24 hours or so, i'll >> commit the change to the 3.8 branch and the trunk. > > The change looks harmless, but please verify it passes the ZODB tests. it does. > You should also add a test that currently fails, but passes with your > change; that's the best way to ensure it won't break again. I recognize > that may be fairly difficult, but there are a lot of people here who > will answer questions. no problem, this one was easy: http://svn.zope.org/ZODB/trunk/src/ZODB/blob.py?rev=91065&view=rev -r ___ 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] problems w/ Pickle based deep-copy of Blob objects
Rob Miller wrote: > i'm assuming from the lack of response that nobody has a problem w/ this > change... if i don't hear differently within the next 24 hours or so, i'll > commit the change to the 3.8 branch and the trunk. The change looks harmless, but please verify it passes the ZODB tests. You should also add a test that currently fails, but passes with your change; that's the best way to ensure it won't break again. I recognize that may be fairly difficult, but there are a lot of people here who will answer questions. Shane ___ 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] problems w/ Pickle based deep-copy of Blob objects
i'm assuming from the lack of response that nobody has a problem w/ this change... if i don't hear differently within the next 24 hours or so, i'll commit the change to the 3.8 branch and the trunk. here's the patch: Index: src/ZODB/blob.py === --- src/ZODB/blob.py(revision 91033) +++ src/ZODB/blob.py(working copy) @@ -93,7 +93,7 @@ # XXX should we warn of this? Maybe? if self._p_changed is None: return -for ref in self.readers+self.writers: +for ref in (self.readers or [])+(self.writers or []): f = ref() if f is not None: f.close() -r Rob Miller wrote: > hi all, > > i've been experimenting w/ getting ZODB 3.8's blob storage support to work w/ > the openplans.org stack, and have hit a snag about which i could use some > feedback. > > the problem is that CMFEditions, which we use to version our content, uses > Pickler.dump() and Unpickler.load() to create deep copies of objects, which > then get stored as historical versions of those objects. the heart of the > issue is probably best illustrated by an interactive session: > > >>> blob > > >>> blob.readers > [] > >>> blob.writers > [] > >>> stream = StringIO() > >>> p = Pickler(stream, 1) > >>> p.dump(blob) > > >>> u = Unpickler(stream) > >>> stream.seek(0) > >>> copy = u.load() > >>> copy > > >>> copy.readers > >>> copy.writers > > 'readers' and 'writers' are defined as None on the Blob class, but they get > initialized as empty list instance variables by the __init__ and __setstate__ > methods. neither of these are triggered by the deep copy process, however, > and apparently the __getstate__ and __setstate__ implementations prevent > these > instance variables from propagating to the pickled copy. > > this seems okay, at first, but if anything is done w/ the transaction that > triggers the copy's _p_invalidate method to be called, it blows up with a > TypeError on the following: > > for ref in self.readers+self.writers: > > the problem seems to go away w/ a simple change: > > for ref in (self.readers or []) + (self.writers or []): > > this seems safe to me; it's not actually changing the state of any objects, > and it's only going to have an impact when an object w/ no readers and > writers > is trying to be invalidated. but i don't know enough about what's going on > here to feel confident that this is the right solution, that i'm not > introducing some subtle ZODB bug that's going to bite me (or someone else) > down the road. > > can anyone tell me whether or not this seems safe? if this isn't the right > approach, what would you suggest? (my second guess would be to fix the > __getstate__ and __setstate__ implementation so that these instance variables > make it through the pickling process in the first place...) > > any help very appreciated, thanks! > > -r ___ 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
