new BTreeContainer implementation, please review
Hi, While fixing some bugs in zope.app.container, I've also modified the implementation of the BTreeContainer, by not inheriting from the SampleContainer, and directly accessing the btree. This had remained as a TODO in the btree.py file, so I did it, but... The result is all previous persisted BTreeContainers (such as PrincipalFolder) are broken because the btree used to be stored in self._SampleContainer__data, while the new implementation stores it in self._BTreeContainer__data. So I've added a property to offer a transparent backward compatibility: def _get__data(self): try: return self._BTreeContainer__data except: return self._SampleContainer__data def _set__data(self, value): try: self._BTreeContainer__data = value except: self._SampleContainer__data = value def _del_data(self): try: del self._BTreeContainer__data except: del self._SampleContainer__data __data = property(_get__data, _set__data, _del_data) Do you think it is safe? Is there any better solution for this? Should I rather write an evolution script? Or should I revert all this back to inheriting from SampleContainer? Christophe
On Fri, Jun 13, 2008 at 3:40 PM, Christophe Combelles <ccomb@free.fr> wrote:
While fixing some bugs in zope.app.container, I've also modified the implementation of the BTreeContainer, by not inheriting from the SampleContainer, and directly accessing the btree. This had remained as a TODO in the btree.py file, so I did it, but...
The result is all previous persisted BTreeContainers (such as PrincipalFolder) are broken because the btree used to be stored in self._SampleContainer__data, while the new implementation stores it in self._BTreeContainer__data.
So I've added a property to offer a transparent backward compatibility:
[snip]
Do you think it is safe? Is there any better solution for this? Should I rather write an evolution script? Or should I revert all this back to inheriting from SampleContainer?
Another option would be to leave the attribute with the "wrong" name and just add a note that it's for backward compatible. -- Benji York Senior Software Engineer Zope Corporation
On Fri, Jun 13, 2008 at 09:40:06PM +0200, Christophe Combelles wrote:
While fixing some bugs in zope.app.container, I've also modified the implementation of the BTreeContainer, by not inheriting from the SampleContainer, and directly accessing the btree. This had remained as a TODO in the btree.py file, so I did it, but...
The result is all previous persisted BTreeContainers (such as PrincipalFolder) are broken because the btree used to be stored in self._SampleContainer__data, while the new implementation stores it in self._BTreeContainer__data.
So I've added a property to offer a transparent backward compatibility:
def _get__data(self): try: return self._BTreeContainer__data except:
Please do not use bare except clauses. Replace this with except AttributeError:
return self._SampleContainer__data def _set__data(self, value): try: self._BTreeContainer__data = value except:
When could this ever fail?
self._SampleContainer__data = value def _del_data(self): try: del self._BTreeContainer__data except: del self._SampleContainer__data
Do we need __del__ at all?
__data = property(_get__data, _set__data, _del_data)
Do you think it is safe?
Not if you want compatibility in both directions. If you want databases created with the new zope.app.container to be readable with the old zope.app.container (consider, e.g. a developer using a sandbox and switching between bleeding-edge development and old maintenance branches), then you can't do this.
Is there any better solution for this?
Perhaps a __setstate__ method to rename the attribute? See the PersistentMapping class in persistent/mapping.py for an example.
Should I rather write an evolution script?
*loathes*
Or should I revert all this back to inheriting from SampleContainer?
Or you could do like Benji York suggested and always use the backwards-compatible name _SampleContainer__data. (Why oh why did SampleContainer want to use a double underscored name?) Marius Gedminas -- Of course I use Microsoft. Setting up a stable unix network is no challenge ;p
Marius Gedminas a écrit :
On Fri, Jun 13, 2008 at 09:40:06PM +0200, Christophe Combelles wrote:
While fixing some bugs in zope.app.container, I've also modified the implementation of the BTreeContainer, by not inheriting from the SampleContainer, and directly accessing the btree. This had remained as a TODO in the btree.py file, so I did it, but...
The result is all previous persisted BTreeContainers (such as PrincipalFolder) are broken because the btree used to be stored in self._SampleContainer__data, while the new implementation stores it in self._BTreeContainer__data.
So I've added a property to offer a transparent backward compatibility:
def _get__data(self): try: return self._BTreeContainer__data except:
Please do not use bare except clauses. Replace this with
except AttributeError:
return self._SampleContainer__data def _set__data(self, value): try: self._BTreeContainer__data = value except:
When could this ever fail?
self._SampleContainer__data = value def _del_data(self): try: del self._BTreeContainer__data except: del self._SampleContainer__data
Do we need __del__ at all?
__data = property(_get__data, _set__data, _del_data)
Do you think it is safe?
Not if you want compatibility in both directions. If you want databases created with the new zope.app.container to be readable with the old zope.app.container (consider, e.g. a developer using a sandbox and switching between bleeding-edge development and old maintenance branches), then you can't do this.
Is there any better solution for this?
Perhaps a __setstate__ method to rename the attribute? See the PersistentMapping class in persistent/mapping.py for an example.
Should I rather write an evolution script?
*loathes*
Or should I revert all this back to inheriting from SampleContainer?
Or you could do like Benji York suggested and always use the backwards-compatible name _SampleContainer__data.
thanks for the suggestions! I will do what Benji told, this seems the safest solution, and will allow to backport it to the 3.5 branch, so that the btree length calculation could also be fixed in zope 3.4. Christophe
(Why oh why did SampleContainer want to use a double underscored name?)
Marius Gedminas
------------------------------------------------------------------------
_______________________________________________ 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 )
participants (3)
-
Benji York -
Christophe Combelles -
Marius Gedminas