[Zope-dev] new BTreeContainer implementation, please review

[Marius Gedminas]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mail.zope.org/pipermail/zope-dev/attachments/20080613/b20a138a/attachment.bin