Re: [Zope-dev] Adding broken/missing support to zope.interface?
Hanno Schlichting ha...@hannosch.eu writes: On Mon, Apr 9, 2012 at 10:33 PM, Tres Seaver tsea...@palladion.com wrote: Persistent component registries are not a good enough reason to add such coupling (I'd be in favor of splitting support for persistent registries out of zope.component, too, while we're at it). Let's put the broken support into code which depends on zope.interface, zope.component, and the ZODB, and invert the dependency by having the new code install something into the base code which provides the desired support: the only change to zope.interface should be documenting the insertion point, and testing that it does the right thing when a dummy is plugged into it. I looked at the possible contenders for that dependency description. The ZODB depends on zope.interface itself, though not on zope.component. zope.container is the one that has the most minimal dependencies, while still relying on zope.component and the ZODB. zope.site depends on zope.container, but given its scope sounds like the better place to me. I vaguely remember us discussing to move persistent registries into zope.site at some point. Since we moved zope.site.hooks into zope.component, zope.site doesn't have much else to do anymore. Apart from those two, there's a whole lot more that have far more dependencies or are unrelated in scope, like zope.annotation or zope.catalog. This problem isn't so much ZODB specific as it is specific to pickling. The problem I don't know how to solve without modifying zope.interface is that the on pickle end of things, the only hook I'm aware of is on the unpickling side, namely overriding `find_global` as ZODB does. But there's no way for `find_global` to know that the given global should be an interface just from the module and name which is what the pickle contains. We need to hook into the process at the time the object is pickled. As far as I can see the only way to do that is through the object's __reduce__ method. As such, the only options I see are to add something conditional to `zope.interface.InterfaceClass.__reduce__` or to make `zope.interface.InterfaceClass.__reduce__` hookable in some way. Would the latter address the concerns people are raising here? If so, what's the right way to approach implement that? Just monkey patching from ZODB to zope.interface? Ross ___ 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 )
Re: [Zope-dev] Adding broken/missing support to zope.interface?
On Wed, Apr 11, 2012 at 09:30:36AM -0700, Ross Patterson wrote: Hanno Schlichting ha...@hannosch.eu writes: On Mon, Apr 9, 2012 at 10:33 PM, Tres Seaver tsea...@palladion.com wrote: Persistent component registries are not a good enough reason to add such coupling (I'd be in favor of splitting support for persistent registries out of zope.component, too, while we're at it). Let's put the broken support into code which depends on zope.interface, zope.component, and the ZODB, and invert the dependency by having the new code install something into the base code which provides the desired support: the only change to zope.interface should be documenting the insertion point, and testing that it does the right thing when a dummy is plugged into it. I looked at the possible contenders for that dependency description. The ZODB depends on zope.interface itself, though not on zope.component. zope.container is the one that has the most minimal dependencies, while still relying on zope.component and the ZODB. zope.site depends on zope.container, but given its scope sounds like the better place to me. I vaguely remember us discussing to move persistent registries into zope.site at some point. Since we moved zope.site.hooks into zope.component, zope.site doesn't have much else to do anymore. Apart from those two, there's a whole lot more that have far more dependencies or are unrelated in scope, like zope.annotation or zope.catalog. This problem isn't so much ZODB specific as it is specific to pickling. The problem I don't know how to solve without modifying zope.interface is that the on pickle end of things, the only hook I'm aware of is on the unpickling side, namely overriding `find_global` as ZODB does. But there's no way for `find_global` to know that the given global should be an interface just from the module and name which is what the pickle contains. We need to hook into the process at the time the object is pickled. As far as I can see the only way to do that is through the object's __reduce__ method. As such, the only options I see are to add something conditional to `zope.interface.InterfaceClass.__reduce__` or to make `zope.interface.InterfaceClass.__reduce__` hookable in some way. Would the latter address the concerns people are raising here? Tres was suggesting something like that. It would address my concerns. If so, what's the right way to approach implement that? I think you need someone intimate with the ZODB to suggest that. One way might be something like the adapter_hooks already present in interface.py. That would at least be consistent, but there are probably many good reasons to not do it that way. That code could look something like: reduce_hooks = [] class InterfaceClass: def __reduce__(self): for hook in reduce_hooks: result = hook(self) if result is not None: return result return self.__name__ Just monkey patching from ZODB to zope.interface? Indeed, that is also another option. Ross ___ 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 ) -- Brian Sutherland ___ 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 )
Re: [Zope-dev] Adding broken/missing support to zope.interface? (was: experimental.broken - Graceful handling of broken interfaces and components in the ZODB)
On Mon, Apr 09, 2012 at 08:15:16PM +, Martin Aspeli wrote: On 9 April 2012 15:41, Brian Sutherland br...@vanguardistas.net wrote: 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. It's a soft dependency only, looking at the code sample. Soft dependencies are cheating ;) Cheating is great when you are refactoring old code to try to minimize dependencies. For new code it's better to just do it right. 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. It doesn't add any more (required) dependencies. It is required the moment you actually want to write a test for your code. That means it's pretty much required for development. Also, if you of the more fanatical sort that believe you should run what you test and test what you run then even a testing dependency is a real dependency. This is a real issue that is breaking the sites of a lot of real users of zope.interface in a way that is hard to debug and reverse. It is quite ridiculous that something as fundamental as zope.interface needs to have knowledge of a specific database implementation to avoid problems that are hard to debug and reverse. Can you think of a better way to get around it? Other than don't get into the situation which isn't a valid answer as long as the ZTK ecosystem supports persistent local components. I'm sorry, having never actually used the ZODB I have absolutely no idea on what level this could best be implemented. A question, do you need this code enabled all the time? Or is it something you only need during specific moments like upgrades? -- Brian Sutherland ___ 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 )
Re: [Zope-dev] Adding broken/missing support to zope.interface? (was: experimental.broken - Graceful handling of broken interfaces and components in the ZODB)
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 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
Re: [Zope-dev] Adding broken/missing support to zope.interface? (was: experimental.broken - Graceful handling of broken interfaces and components in the ZODB)
On 9 April 2012 15:41, Brian Sutherland br...@vanguardistas.net wrote: 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. It's a soft dependency only, looking at the code sample. 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. It doesn't add any more (required) dependencies. This is a real issue that is breaking the sites of a lot of real users of zope.interface in a way that is hard to debug and reverse. Can you think of a better way to get around it? Other than don't get into the situation which isn't a valid answer as long as the ZTK ecosystem supports persistent local components. Martin ___ 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 )
Re: [Zope-dev] Adding broken/missing support to zope.interface?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/09/2012 04:15 PM, Martin Aspeli wrote: On 9 April 2012 15:41, Brian Sutherland br...@vanguardistas.net wrote: 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 Agreeed. I'm more like -20 on this implementation, but +1 on the goal. 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. It's a soft dependency only, looking at the code sample. 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. It doesn't add any more (required) dependencies. - -1 on any dependency, soft or hard, from zope.interface - ZODB This is a real issue that is breaking the sites of a lot of real users of zope.interface in a way that is hard to debug and reverse. Can you think of a better way to get around it? Other than don't get into the situation which isn't a valid answer as long as the ZTK ecosystem supports persistent local components. Persistent component registries are not a good enough reason to add such coupling (I'd be in favor of splitting support for persistent registries out of zope.component, too, while we're at it). Let's put the broken support into code which depends on zope.interface, zope.component, and the ZODB, and invert the dependency by having the new code install something into the base code which provides the desired support: the only change to zope.interface should be documenting the insertion point, and testing that it does the right thing when a dummy is plugged into it. Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software Excellence by Designhttp://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+DR4gACgkQ+gerLs4ltQ5THACgzvZtoksPW/V30TsR3pWa7PyY WUwAniRY51tIHXS1ohd/6K+TkZZwy+7A =kdaO -END PGP SIGNATURE- ___ 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 )
Re: [Zope-dev] Adding broken/missing support to zope.interface?
On Mon, Apr 9, 2012 at 4:33 PM, Tres Seaver tsea...@palladion.com wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/09/2012 04:15 PM, Martin Aspeli wrote: On 9 April 2012 15:41, Brian Sutherland br...@vanguardistas.net wrote: 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 Agreeed. I'm more like -20 on this implementation, but +1 on the goal. 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. It's a soft dependency only, looking at the code sample. 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. It doesn't add any more (required) dependencies. - -1 on any dependency, soft or hard, from zope.interface - ZODB This is a real issue that is breaking the sites of a lot of real users of zope.interface in a way that is hard to debug and reverse. Can you think of a better way to get around it? Other than don't get into the situation which isn't a valid answer as long as the ZTK ecosystem supports persistent local components. Persistent component registries are not a good enough reason to add such coupling (I'd be in favor of splitting support for persistent registries out of zope.component, too, while we're at it). Let's put the broken support into code which depends on zope.interface, zope.component, and the ZODB, and invert the dependency by having the new code install something into the base code which provides the desired support: the only change to zope.interface should be documenting the insertion point, and testing that it does the right thing when a dummy is plugged into it. +1 Jim -- Jim Fulton http://www.linkedin.com/in/jimfulton Jerky is better than bacon! http://www.dublinstore.com/ ___ 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 )
Re: [Zope-dev] Adding broken/missing support to zope.interface?
On Mon, Apr 9, 2012 at 10:33 PM, Tres Seaver tsea...@palladion.com wrote: Persistent component registries are not a good enough reason to add such coupling (I'd be in favor of splitting support for persistent registries out of zope.component, too, while we're at it). Let's put the broken support into code which depends on zope.interface, zope.component, and the ZODB, and invert the dependency by having the new code install something into the base code which provides the desired support: the only change to zope.interface should be documenting the insertion point, and testing that it does the right thing when a dummy is plugged into it. I looked at the possible contenders for that dependency description. The ZODB depends on zope.interface itself, though not on zope.component. zope.container is the one that has the most minimal dependencies, while still relying on zope.component and the ZODB. zope.site depends on zope.container, but given its scope sounds like the better place to me. I vaguely remember us discussing to move persistent registries into zope.site at some point. Since we moved zope.site.hooks into zope.component, zope.site doesn't have much else to do anymore. Apart from those two, there's a whole lot more that have far more dependencies or are unrelated in scope, like zope.annotation or zope.catalog. Hanno ___ 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] Adding broken/missing support to zope.interface? (was: experimental.broken - Graceful handling of broken interfaces and components in the ZODB)
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
Re: [Zope-dev] Adding broken/missing support to zope.interface? (was: experimental.broken - Graceful handling of broken interfaces and components in the ZODB)
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
