Ian, I appreciate your skepticism, and think that it is healthy to question and discuss the issues. The open discussion helps all of us to better understand the software, and to better organize any changes that do get in.
That being said, there are two separate issues now as I see it: -- first issue: subclassing ThreadedAppServer -- The ability to Subclass the ThreadedAppServer which I think is a good thing: a) Subclassing continues along with the paradigm that AppServer already uses, and allows further refinement of the operation by a subclass. b) Subclassing is a well understood OO paradigm of extending or altering functionality of a class. I think the MixIn approach is pretty cool, and offers some advantages over subclassing. In particular as Ian points out, the ability to have separate modules both alter the behavior of different parts of a class. But that doesn't mean it should be used instead of subclassing where subclassing is the logical choice. On this topic, I think some minor changes to the run and main functions within ThreadedAppServer.py will make it very easy to subclass ThreadedAppServer. Furthermore, if these methods (in particular 'run') were placed on the abstract baseclass AppServer, you would have a very clean model for subclassing and starting an appserver. Create an instance of the specified AppServer, and then call the run method. -- second issue: addContext() not calling contextInitialize() -- While testing the MixIn alternative to subclassing ThreadedAppServer, I discovered that addContext() method does not call contextInitialize() if the module is already loaded. There is a check for sys.modules.has_key(importAsName) and if this is set, the contextInitialized() function will not be called. This could be true if your particular module had been imported by some other module. I suggest that the contextInitialized() method should be called if it exists, and the name is not in self._contexts. This way, other modules are free to import items under a kit, and contextInitialize() will always be called only once when the context is added. I have attached a diff which I believe solves the addContext() problem. It also outputs an error if there was an ImportError. Prior behavior was to simply ignore this condition. Arguably, an ImportError might be something that should result in the AppServer failing to load. -Stuart- >On Mon, 2002-11-18 at 20:51, Stuart Donaldson wrote: > > >>Ok, so I can see the value of MixIn() in some cases rather than subclassing, >>depending on what you are doing. However in my case, I wanted to replace the >>createApplication method on ThreadedAppServer, and I tried doing this in the >>__init__ method of my module. >> >>I need to import my module prior to starting the ThreadedAppServer because I >>want to affect how the ThreadedAppServer starts. If I do this, then the >>Application method addContext() fails to call contextInitialize() because >>the module has already been loaded. >> >> > >If you want to change the session class, then maybe you should change >that class directly instead of trying to change the AppServer. If you >want to create an entirely new session class, I can understand the >problem. Even then, couldn't you modify the instance variable of the >AppServer after the fact? > >I mean, *I'm* not opposed to making local subclassing possible, and I >don't think anyone else is (like you say, most of the code is already >there with this in mind) -- but I'm also not sure it's necessary. It's >purely in a desire to be minimal that I'm being skeptical. > >You mentioned sessions before -- what specifically do you want to do? >I.e., which class do you want to subclass (since there's a lot of >candidates)? > > > >>Perhaps addContext() should be maintaining a list of contexts which have >>been initialized, rather than just initializing them if addContext() is the >>actual place where they are loaded? >> >> > >I don't follow this, though. If you're looking for something with >somewhat different semantics than addContext, maybe a plugin that uses >InstallInWebKit(appServer)? > > Ian >
appl.diff
Description: Binary data
