[Twisted-Python] Interfaces and adapters
Let's say we defined an interface and an adapter:
class IFoo(zope.interfaces.Interface): pass
twisted.python.components.registerAdapter(Foo, Bar, IFoo)
As it is for now, the registerAdapter accepts any Foo, without regarding what it implements.
Could somebody give any rationale why this condition shouldn't always hold:
foo = IFoo(bar) assert IFoo.providedBy(foo)
Seems it is not unreasonable to expect this behaviour, because it is exactly what interfaces are all about, or am I wrong?
On Fri, Aug 26, 2005 at 10:17:07AM +0400, en.karpachov@ospaz.ru wrote:
Let's say we defined an interface and an adapter:
class IFoo(zope.interfaces.Interface): pass
twisted.python.components.registerAdapter(Foo, Bar, IFoo)
As it is for now, the registerAdapter accepts any Foo, without regarding what it implements.
It has to accept pretty much anything -- adapters can be e.g. functions as well as classes.
Could somebody give any rationale why this condition shouldn't always hold:
foo = IFoo(bar) assert IFoo.providedBy(foo)
Seems it is not unreasonable to expect this behaviour, because it is exactly what interfaces are all about, or am I wrong?
That's a pretty reasonable expectation. I think emitting a warning if this doesn't hold would be a sane thing to do, and it wouldn't break any code.
-Andrew.
On Aug 25, 2005, at 11:30 PM, Andrew Bennetts wrote:
On Fri, Aug 26, 2005 at 10:17:07AM +0400, en.karpachov@ospaz.ru wrote:
Let's say we defined an interface and an adapter:
class IFoo(zope.interfaces.Interface): pass
twisted.python.components.registerAdapter(Foo, Bar, IFoo)
As it is for now, the registerAdapter accepts any Foo, without regarding what it implements.
It has to accept pretty much anything -- adapters can be e.g. functions as well as classes.
Could somebody give any rationale why this condition shouldn't always hold:
foo = IFoo(bar) assert IFoo.providedBy(foo)
Seems it is not unreasonable to expect this behaviour, because it is exactly what interfaces are all about, or am I wrong?
That's a pretty reasonable expectation. I think emitting a warning if this doesn't hold would be a sane thing to do, and it wouldn't break any code.
It would certainly slow it down, though. I think this is a "consenting adults" kind of scenario. If someone registers an adapter, they probably didn't do it by accident!
-bob
On Fri, 26 Aug 2005 16:30:26 +1000 Andrew Bennetts wrote:
Could somebody give any rationale why this condition shouldn't always hold:
foo = IFoo(bar) assert IFoo.providedBy(foo)
Seems it is not unreasonable to expect this behaviour, because it is exactly what interfaces are all about, or am I wrong?
That's a pretty reasonable expectation. I think emitting a warning if this doesn't hold would be a sane thing to do, and it wouldn't break any code.
For me, raising CannotAdapt would be better choice. After all, getting an object that implements requested interface is the very thing what adaptation is used for.
If I, say, adapt WovenContext to ISession, I get a session object which does not implement the ISession interface itself. Absurdly.
On Fri, Aug 26, 2005 at 12:00:39PM +0400, en.karpachov@ospaz.ru wrote:
On Fri, 26 Aug 2005 16:30:26 +1000 Andrew Bennetts wrote:
Could somebody give any rationale why this condition shouldn't always hold:
foo = IFoo(bar) assert IFoo.providedBy(foo)
Seems it is not unreasonable to expect this behaviour, because it is exactly what interfaces are all about, or am I wrong?
That's a pretty reasonable expectation. I think emitting a warning if this doesn't hold would be a sane thing to do, and it wouldn't break any code.
For me, raising CannotAdapt would be better choice. After all, getting an object that implements requested interface is the very thing what adaptation is used for.
If I, say, adapt WovenContext to ISession, I get a session object which does not implement the ISession interface itself. Absurdly.
Yep. But that's a trivially fixable bug in that adapter. And this is exactly why raising an exception would be bad -- it will break code that otherwise works fine. Even though such code is arguably buggy, Twisted tries reasonably hard not to break backwards compatibility without warning. We couldn't raise CannotAdapt here without first giving DeprecationWarnings for at least one release.
-Andrew.
On Fri, Aug 26, 2005 at 12:54:47AM -0700, Bob Ippolito wrote:
On Aug 25, 2005, at 11:30 PM, Andrew Bennetts wrote:
[...]
That's a pretty reasonable expectation. I think emitting a warning if this doesn't hold would be a sane thing to do, and it wouldn't break any code.
It would certainly slow it down, though. I think this is a "consenting adults" kind of scenario. If someone registers an adapter, they probably didn't do it by accident!
Right. The bug the OP has here isn't that the adapter shouldn't have been registered, but that the adapter fails to properly declare what interfaces it provides, so that the result of IBar(IFoo(x)) will fail even though x -> IFoo and IFoo -> IBar adapters are registered.
The performance question is important... benchmarks of any changes here would be a good idea.
A weaker solution would be to only check at registration time, but that's uglier, because it only works for adapters that are classes, and that don't have creative __new__ methods. That's almost certainly 99% of adapters, but ugly and incomplete solutions make me nervous.
Anyway, the underlying zope.interface machinery allows this -- and includes doctest documentation that relies on this (it registers *ints* as adapters, iirc!). I'm inclined to agree with your "consenting adults" approach, but I don't feel strongly either way.
-Andrew.
On Fri, 26 Aug 2005 18:40:07 +1000 Andrew Bennetts wrote:
On Fri, Aug 26, 2005 at 12:00:39PM +0400, en.karpachov@ospaz.ru wrote:
If I, say, adapt WovenContext to ISession, I get a session object which does not implement the ISession interface itself. Absurdly.
Yep. But that's a trivially fixable bug in that adapter. And this is
Should it be considered as a bug then?
exactly why raising an exception would be bad -- it will break code that otherwise works fine. Even though such code is arguably buggy, Twisted
One could say that the code in question is already broken. At least, the exception would point to the programmer that they forgot to declare what interfaces are implemented by the adapter.
tries reasonably hard not to break backwards compatibility without warning. We couldn't raise CannotAdapt here without first giving DeprecationWarnings for at least one release.
Well, if raising CannotAdapt is not appropriate, then probably some decorator to AdapterFactory could help? Something like (pseudocode):
def affirm(adapterFactory, interface):
def safe_factory(orig): adapter = adapterFactory(orig) if not interface.providedBy(adapter): warn("Failed to adapt") adapter = MakeTheObjectToProvide(adapter, interface) return adapter
return safe_factory
adapterFactory = affirm(adapterFactory, interface)
On Fri, 2005-08-26 at 16:30 +1000, Andrew Bennetts wrote:
As it is for now, the registerAdapter accepts any Foo, without regarding what it implements.
It has to accept pretty much anything -- adapters can be e.g. functions as well as classes.
Could somebody give any rationale why this condition shouldn't always hold:
foo = IFoo(bar) assert IFoo.providedBy(foo)
Seems it is not unreasonable to expect this behaviour, because it is exactly what interfaces are all about, or am I wrong?
If the adapter is a function, the function may not implement the interface. The actual expected behavior is that the result of *calling* the adapter *provides* the interface. An adapter class may well implement the interface in order for this to happen, but it's not a necessary condition.
On Fri, 26 Aug 2005 12:12:02 -0400 Itamar Shtull-Trauring wrote:
Could somebody give any rationale why this condition shouldn't always hold:
foo = IFoo(bar) assert IFoo.providedBy(foo)
Seems it is not unreasonable to expect this behaviour, because it is exactly what interfaces are all about, or am I wrong?
If the adapter is a function, the function may not implement the interface. The actual expected behavior is that the result of *calling* the adapter *provides* the interface. An adapter class may well implement the interface in order for this to happen, but it's not a necessary condition.
That's true, and I don't see how does it differ from my pseudocode example. And neither the twisted.web.server.Session class implements, nor it's objects provides the ISession interface.
On Sat, 2005-08-27 at 01:17 +0400, en.karpachov@ospaz.ru wrote:
If the adapter is a function, the function may not implement the interface. The actual expected behavior is that the result of *calling* the adapter *provides* the interface. An adapter class may well implement the interface in order for this to happen, but it's not a necessary condition.
That's true, and I don't see how does it differ from my pseudocode example.
Your pseudo-code does indeed match what I said, and would be useful to have. I have a bug open for "mode of running that is slower but does a lot more runtime checking" and this should probably be one of the features.
Adaptation is slow enough as it is. Enforcing this convention would be a fairly substantial cost for something which is already multiplying the cost of what would normally be a function call by tens or hundreds.
Not enforcing it is also in the spirit of Python's "language for consenting adults". If your object really doesn't implement the interface, the unit tests will catch it, right? ;-)
I do hope that we eventually have a "pedantic mode" where Twisted will spew copious warnings about things like this, but in the default case it's just not worth the effort to dot every i and cross every |.
On Fri, 26 Aug 2005 23:22:38 -0400 glyph@divmod.com wrote:
Adaptation is slow enough as it is. Enforcing this convention would be a fairly substantial cost for something which is already multiplying the cost of what would normally be a function call by tens or hundreds.
Not enforcing it is also in the spirit of Python's "language for consenting adults". If your object really doesn't implement the interface, the unit tests will catch it, right? ;-)
For an object I own, yes, that's right. For someone else's objects, I'm not sure how to deal with them.
The question is, if the object doesn't declare itself provides the interface it adapts to, should this be considered as a bug? If so, then we have it with twisted.web.server.Session and nevow.guard.GuardSession (and probably the same situation with twisted.web2).
There is another problem with it. The twisted.web.server.Session cannot declare itself implementing nevow.inevow.ISession as it doesn't know anything about nevow; but nevow registers it as default adapter from WovenContext to inevow.ISession. So we have a situation:
session = inevow.ISession(ctx) assert inevow.ISession.providedBy(sess) # oops! it is not
What use in the adaptation above then? It's not really an adapter to the interface we need but just some arbitrary function call, not related to interfaces at all, incidentally looking like an adapter.
Probably, a decorator to the adapterFactory could help; even if all got a bit slower, we'll get more correct behaviour. Anyway, is it that slower? I've heard the nevow as well as Python are not about speed, but about building robust applications.
en.karpachov@ospaz.ru wrote:
The question is, if the object doesn't declare itself provides the interface it adapts to, should this be considered as a bug? If so, then we have it with twisted.web.server.Session and nevow.guard.GuardSession (and probably the same situation with twisted.web2).
twisted.web2 is yet to come and also nevow working on it although there's a branch in nevow that works with it right now. So everything is in flux and yet to be decided.
There is another problem with it. The twisted.web.server.Session cannot declare itself implementing nevow.inevow.ISession as it doesn't know anything about nevow; but nevow registers it as default adapter from WovenContext to inevow.ISession. So we have a situation:
session = inevow.ISession(ctx) assert inevow.ISession.providedBy(sess) # oops! it is no
What use in the adaptation above then? It's not really an adapter to the interface we need but just some arbitrary function call, not related to interfaces at all, incidentally looking like an adapter.
Woah, wait a bit :) If you want to really use sessions in nevow you have to use guard, if you use guard the session object will be GuardSession that knows about nevow and can implement inevow.ISession. Currently GuardSession doesn't implement inevow.ISession but will in a sec when I commit the fix.
If you don't use guard you are really getting a server.Session object from twisted.web returned by nevow.appserver.sessionFactory that calls NevowRequest.getSession() which will return the GuardSession if you are using guard or the other one if not (as I already said). Oh, server.Session does suck.
Probably, a decorator to the adapterFactory could help; even if all got a bit slower, we'll get more correct behaviour. Anyway, is it that slower? I've heard the nevow as well as Python are not about speed, but about building robust applications.
True, but doing more than what is really needed doesn't do good anyway. Losing speed for free is not something you really wish.
On Sat, 27 Aug 2005 13:47:58 +0200 Valentino Volonghi aka Dialtone wrote:
twisted.web2 is yet to come and also nevow working on it although there's a branch in nevow that works with it right now. So everything is in flux and yet to be decided.
Of course, I'm aware of that but I couldn't help but try it right now :)
If you want to really use sessions in nevow you have to use guard, if you use guard the session object will be GuardSession that knows about nevow and can implement inevow.ISession. Currently GuardSession doesn't implement inevow.ISession but will in a sec when I commit the fix.
Good news, thanks! I'd like to operate with interfaces, not with implementation classes:
registerAdapter(MyUserInfo, ISession, IMyUserInfo)
If you don't use guard you are really getting a server.Session object from twisted.web returned by nevow.appserver.sessionFactory that calls NevowRequest.getSession() which will return the GuardSession if you are using guard or the other one if not (as I already said). Oh, server.Session does suck.
Ok. I didn't know that using guard is mandatory for sessions in the Nevow, but now I do. Deprecation warning about twisted.web.server.Session as the default adapter would be nice, eventually.
Thanks for the suggestion.
On Sat, Aug 27, 2005 at 01:09:21PM +0400, en.karpachov@ospaz.ru wrote: [...]
There is another problem with it. The twisted.web.server.Session cannot declare itself implementing nevow.inevow.ISession as it doesn't know anything about nevow; but nevow registers it as default adapter from WovenContext to inevow.ISession. So we have a situation:
zope.interface allows for this case -- nevow.inevow just needs to say:
zope.interface.classImplements(twisted.web.server.Session, ISession)
-Andrew.
On Sun, 28 Aug 2005 02:07:09 +1000 Andrew Bennetts wrote:
zope.interface allows for this case -- nevow.inevow just needs to say:
zope.interface.classImplements(twisted.web.server.Session, ISession)
Good idea. If the Nevow did this workaround at it's startup time, it would be useful.
participants (6)
-
Andrew Bennetts -
Bob Ippolito -
en.karpachov＠ospaz.ru -
glyph＠divmod.com -
Itamar Shtull-Trauring -
Valentino Volonghi aka Dialtone