I think I agree with both of your points -- allowing subclassing of
ThreadedAppServer is a good idea, and also your point about calling
contextInitialize in addContext.
Can you upload your diffs to the SourceForge patch manager?
> -----Original Message-----
> From: Stuart Donaldson [mailto:stuartd@...]
> Sent: Tuesday, November 19, 2002 10:32 AM
> To: 'Ian Bicking'
> Cc: Webware devel
> Subject: RE: [Webware-devel] RE: Developer access to WebKit module
> 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.
> >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
> >>depending on what you are doing. However in my case, I
> wanted to replace
> >>createApplication method on ThreadedAppServer, and I tried
> doing this in
> >>__init__ method of my module.
> >>I need to import my module prior to starting the
> ThreadedAppServer because
> >>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
> >>Perhaps addContext() should be maintaining a list of
> contexts which have
> >>been initialized, rather than just initializing them if
> addContext() is
> >>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
> > Ian