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 <m...@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 <m...@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 <m...@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@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@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@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 )