Re: Adding broken/missing support to zope.interface? (was: experimental.broken - Graceful handling of broken interfaces and components in the ZODB)
Brian Sutherland <[email protected]>
| Newsgroups | gmane.comp.web.zope.devel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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 <[email protected]> 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 - [email protected] > > 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 - [email protected] > 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 _______________________________________________ Zope-Dev maillist - [email protected] 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 )