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

Mon Apr 9 15:41:11 UTC 2012

On Sun, Apr 08, 2012 at 01:04:37PM -0700, Ross Patterson 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))
>     ...

-1

For a number of reasons, but mainly because you add a test dependency on
the ZODB from zope.interface. I think that zope.interface is such a
fundamental package and the dependency is unacceptable.

There has lately been a LOT of work done reducing the dependency
structure of zope.* packages. You need a VERY good reason to start
reversing that.

> 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 )

-- 
Brian Sutherland