From: Stuart D. <st...@al...> - 2002-11-19 02:51:29
|
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. 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? -Stuart- > -----Original Message----- > [mailto:web...@li...]On Behalf Of Ian > Bicking > ... > It's not so much a Python thing. I originally thought > subclassing would > be best, but I realized that they it can be awkward in some ways. > > If you want to use local subclassing you could always just change the > import statement to use your local subclass. This won't be entirely > seemless for upgrading, but I don't think that's a big problem. Of > course, you couldn't easily distribute the enhanced class -- > but I think > that's a problem anyway with subclassing. You can distribute a > subclassing of a single class, but that doesn't leave a good > way to use > two customizations (even though they might not otherwise conflict). > > Even if it seems kind of weird, I think the mixin approach solves most > of these -- if you make your customization into a plug-in, it > can change > the classes when its initialized. This should make it easy to > distribute customizations. |
From: Stuart D. <st...@al...> - 2002-11-19 15:32:24
Attachments:
appl.diff
|
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 > |
From: Jason H. <ja...@pe...> - 2002-11-20 16:33:37
|
On Tue, 2002-11-19 at 09:32, Stuart Donaldson wrote: > 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. This sounds good to me. If we can also manage to get main() into the class (instead of being a separate function) it would be even better: Currently, if you do Launch.py ThreadedAppServer it imports the WebKit packages, and looks there for ThreadedAppServer. With small changes, we could teach it to handle something like Launch.py Lib.CustomAppServer which would import the Lib package (assuming it's in your PYTHONPATH), and import the CustomAppServer module from there. If no package is specified we can default to WebKit to preserve backwards-compatibility. -- Jason D. Hildebrand ja...@pe... |
From: Geoffrey T. <gta...@na...> - 2002-11-20 15:48:03
|
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? - Geoff > -----Original Message----- > From: Stuart Donaldson [mailto:st...@al...] > Sent: Tuesday, November 19, 2002 10:32 AM > To: 'Ian Bicking' > Cc: Webware devel > Subject: RE: [Webware-devel] RE: Developer access to WebKit module > > > 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 > > > > |
From: Stuart D. <st...@as...> - 2002-11-20 16:36:34
|
On the ThreadedAppServer subclassing, I have only done what I think is a quick hack right now, adding the class as a parameter to run. I think a nicer more OO solution would be to remove run, and replace it with a start() method on AppServer, along with placing the AppServer stop function on the AppServer class such that it behaves like a static method. This allows subclasses to override the start and stop behavior as well. Then, of course, the other change to fully implement this, is to modify Launch.py, so that a user specified AppServer can be passed in, that is not necessarily part of WebKit. I am working on the assumption that an installed WebKit directory is not writeable by a user, and that they should be able to create their own subclassed AppServer derived from ThreadedAppServer. If you agree that going this extra route would be a good thing, then let me know and I'll give it a shot. For my immediate needs, the hack I made to the run function will allow subclassing of ThreadedAppServer, but not in a very elegant manner. Regarding the addContext() fix, I will upload that today to the SF patch manager. -Stuart- Geoffrey Talvola wrote: >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? > >- Geoff > > |
From: Geoffrey T. <gta...@na...> - 2002-11-21 17:16:19
|
Stuart Donaldson wrote: > On the ThreadedAppServer subclassing, I have only done what I > think is a > quick hack right now, adding the class as a parameter to run. > I think a > nicer more OO solution would be to remove run, and replace it with a > start() method on AppServer, along with placing the AppServer stop > function on the AppServer class such that it behaves like a static > method. This allows subclasses to override the start and stop > behavior > as well. > > Then, of course, the other change to fully implement this, is > to modify > Launch.py, so that a user specified AppServer can be passed > in, that is > not necessarily part of WebKit. I am working on the > assumption that an > installed WebKit directory is not writeable by a user, and that they > should be able to create their own subclassed AppServer derived from > ThreadedAppServer. > > If you agree that going this extra route would be a good > thing, then let > me know and I'll give it a shot. For my immediate needs, the hack I > made to the run function will allow subclassing of ThreadedAppServer, > but not in a very elegant manner. It seems like a good thing. I can't say that I'd actually use it myself, and I'm not sure how many others would use it either. You can decide whether it's worth your while to do the work. - Geoff |
From: Ian B. <ia...@co...> - 2002-11-19 05:05:09
|
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 |