[Zope-dev] Adding broken/missing support to zope.interface? (was: experimental.broken - Graceful handling of broken interfaces and components in the ZODB)

Sun Apr 8 20:52:30 UTC 2012

Not ZCA/ZODB maintainer, but a big +1 from me.

I've also experienced the negative effects of missing code for
(marker) interfaces in persistent data.

Cheers,
Leo

On Sun, Apr 8, 2012 at 22:04, Ross Patterson <me at rpatterson.net> wrote:
> experimental.broken is working well for me.  It greatly aided me in
> getting through a particularly nasty upgrade allowing me to cleanup the
> ZCA cruft left by poorly behaved add-ons.  I'd like to proceed with
> adding the core of this to zope.interface and I need the
> developers/maintainers to weigh in.
>
> The first and most fundamental matter is changing interface pickles such
> that they can be unpickled into something that can fulfill the minimum
> interface contract and don't break the ZCA.  To that end, I'd like to
> add the following to zope.interface.interface:
>
>    ...
>    try:
>        from ZODB.broken import find_global
>        from ZODB.broken import IBroken
>        def find_interface(modulename, globalname,
>                           Broken=IBroken, type=InterfaceClass):
>            """
>            Find a global interface, returning a broken interface if it can't be found.
>            """
>            return find_global(modulename, globalname,
>                               Broken=IBroken, type=InterfaceClass)
>    except ImportError:
>        IBroken = Interface
>        def find_interface(modulename, globalname,
>                           Broken=IBroken, type=InterfaceClass):
>            """
>            Find a global interface, raising ImportError if it can't be found.
>            """
>            # From pickle.Unpickler.find_class
>            __import__(module)
>            mod = sys.modules[module]
>            klass = getattr(mod, name)
>            return klass
>    ...
>    class InterfaceClass(Element, InterfaceBase, Specification):
>    ...
>        def __reduce__(self):
>            if self is IBroken:
>                return self.__name__
>            return (find_interface, (modulename, globalname))
>    ...
>
> With this change, previously existing interface pickles will be
> different from newly committed ones but both pickles would unpickle to
> the same object.  Also, running zodbupdate would convert all pickles to
> the new format.
>
> Is this the correct approach?  If not, how should it be changed or what
> might be the correct way?  Is there a better way to put the broken
> object support in ZODB and still have the same interface pickles when
> using ZODB?
>
> This still leaves the problem of instance provides declarations and
> component registrations which contain previously existing interface
> pickles for missing interfaces.  Without addressing these, previously
> existing instance pickles cannot be unpickled and all component registry
> operations fail.  experimental.broken addresses these by adding handling
> for broken interfaces when provides declaration are unpickled (in the
> ProvidesClass.__init__ method) and when component registries are
> unpickled (in the __setstate__ method of
> persistentregistry.PersistentAdapterRegistry and
> persistentregistry.PersistentComponents).  Again, with these patches,
> missing interfaces don't break instances or registries and allow running
> zodbupdate to fix all existing pickles.  Where should this support live?
> Should I keep it in a separate package, maybe just rename
> experimental.broken to z3c.broken or somesuch?  Should it be merged into
> zope.interface and zope.component?
>
> Will the developers/maintainers of zope.interface, zope.component and/or
> ZODB please weigh in on this and give me feedback towards getting this
> finished?
>
> Thanks!
> Ross
>
> Ross Patterson <me at rpatterson.net> writes:
>
>> I've done some more work on this and I've gotten the component
>> registrations fully working now with one exception that I'm having real
>> trouble with.  I'd like some help with that, more below.  I'm also a bit
>> more clear on what might be appropriate to bring back into
>> zope.interface and I'd like feedback on that.
>>
>> Currently interfaces are pickled as globals.  Given their centrality in
>> any ZCA application, I think they should be pickled using a function:
>>
>>     def __reduce__(self):
>>         return (find_interface, (modulename, globalname))
>>
>> where find_interface, if ZODB.broken.find_global can be imported, in
>> turn calls:
>>
>>     ZODB.broken.find_global(modulename, globalname,
>>                             Broken=IBroken, type=InterfaceClass)
>>
>> since find_global already has nicely abstracted graceful missing global
>> handling.
>>
>> If this were added to zope.interface, and changed ZODB objects with
>> marker interfaces or persistent registries where all the code for the
>> interfaces is still available will then be updated with pickles that
>> will gracefully handle missing interfaces in the future.  Thus you could
>> use zodbupdate to make sure that the interfaces in your ZODB will fail
>> gracefully in the future.  Do you agree/disagree that this belongs in
>> zope.interface?
>>
>> What this will not address are existing interfaces-as-globals pickles
>> where the original interface is now missing.  That's where the other
>> patches in experimental.broken come in, they intercept the two contexts
>> where we can infer that a missing global should be an interface:
>> instance provides declarations and persistent component registries.  By
>> hooking into the unpickling of those objects, we can replace broken
>> classes with broken interfaces as appropriate for those structures.
>> With these patches installed it should be possible to use applications
>> with missing interfaces and to use zodbupdate to make sure that even
>> *already missing* interfaces will fail gracefully both now and in the
>> future.  I think these patches don't belong in zope.interface/component
>> and if they work I would likely move them to five.localsitemanager along
>> with a ZCML file that is not loaded by default.  Does that sound right?
>>
>> Then one thing I haven't been able to get working is making it possible
>> to commit a changed persistent registry when it includes a component
>> registration for a non-persistent component whose class/type is
>> missing.  This works just fine for a *persistent* component whose
>> class/type is missing but fails for a non-persistent component.  The
>> error is:
>>
>>     AttributeError: 'Bar' object has no attribute '__Broken_newargs__'
>>
>> More specifically, '__Broken_newargs__' is set by the Broken.__new__
>> method and I've confirmed that this isn't being called by instrumenting
>> __new__, __init__, and __setstate__.  Only __setstate__ is called when
>> unpickling the non-persistent broken component not __new__ as should
>> be.  Below is the full traceback.  I've also left the package repo in
>> the broken state if you want to examine it, the egg checkout is also a
>> buildout:
>>
>> https://github.com/rpatterson/experimental.broken/commit/a4b648dc78e53c7303ea2d417d2d7c5e7fea24ac
>>
>> Any help with this last issue would be appreciated.
>>
>> Ross
>>
>> Ross Patterson <me at rpatterson.net> writes:
>>
>>> Please take a look at experimental.broken:
>>>
>>> https://github.com/rpatterson/experimental.broken
>>> http://pypi.python.org/pypi/experimental.broken
>>>
>>> The handling of broken objects by the ZODB can make an application with
>>> add-ons that use zope.interface far too fragile.  If marker interfaces
>>> from an add-on are used on objects in the ZODB, removing that add-on can
>>> make any zope.interface operation on that object fail.  Even worse, if
>>> an add-on registers any components in a registry in the ZODB, that
>>> entire registry will become unusable for any ZCA operations which pretty
>>> much breaks everything, including admin interfaces.
>>>
>>> Since the interfaces and the ZCA are often core parts of an application
>>> using the ZODB, it may be appropriate to add special handling for broken
>>> objects to those services.  The experimental.broken patches are my
>>> attempt to prototype such special handling.
>>>
>>> For objects in the ZODB which directly provide a marker interface,
>>> these patches allow that object to behave as without the application
>>> of the marker interface if the interface is no longer available.  If
>>> the interface is made available again, the full behavior of that
>>> interface is restored.  Similarly, if a component whose class,
>>> provided interface, or required interfaces are missing, these patches
>>> allow the registry to perform lookups it would have been able to do
>>> without the broken component registration.  If the component
>>> class, provided interface, and required interfaces are restored,
>>> then the component registration is fully restored.
>>>
>>> If an object or registry in the ZODB is committed to the ZODB with
>>> broken interfaces or components, the commit will succeed and it is still
>>> possible to fully restore previous behavior if the missing classes and
>>> interfaces are restored.  Unfortunately, because interfaces are pickled
>>> as globals, there's no good way to have the same pickle written on
>>> commit for the interface as was in the original pickle, but it should
>>> behave exactly the same.
>>>
>>> The intention of this package is to see if the implementation of broken
>>> object handling is correct and robust enough to merge into
>>> zope.interface and zope.component themselves.  Is this the right
>>> approach?  If not why and what would be better?  How might this approach
>>> be improved?
>>>
>>> Ross
>>>
>>> ------------------------------------------------------------------------------
>>> RSA(R) Conference 2012
>>> Save $700 by Nov 18
>>> Register now
>>> http://p.sf.net/sfu/rsa-sfdev2dev1
>>
>> _______________________________________________
>> Zope-Dev maillist  -  Zope-Dev at zope.org
>> https://mail.zope.org/mailman/listinfo/zope-dev
>> **  No cross posts or HTML encoding!  **
>> (Related lists -
>>  https://mail.zope.org/mailman/listinfo/zope-announce
>>  https://mail.zope.org/mailman/listinfo/zope )
>
> _______________________________________________
> Zope-Dev maillist  -  Zope-Dev at zope.org
> https://mail.zope.org/mailman/listinfo/zope-dev
> **  No cross posts or HTML encoding!  **
> (Related lists -
>  https://mail.zope.org/mailman/listinfo/zope-announce
>  https://mail.zope.org/mailman/listinfo/zope )