Re: [jervlet-patch] Make Jervlet-Jetty accept external configurations

Thanks, I've committed your changes pretty much untouched.  Discussion 
in line.

Johan Sjöberg wrote:

> -Addition of two new Jetty Server creators
> -Configuration logic added to AvalonJettyJervlet to except external 
> configurations
> -Addition of a new AvalonJettyJervlet configuration schema (was missing!)
> -Addition of an example configuration file

I like it

> -Possibility to set the system parameter "jetty.home" (can be 
> considered bad by some)

My opinion is that this should be set through the configuration, as this 
violates IoC and could provide for some unexpected behavior.  I don't 
usually like voodoo :)  More discussion below.

> -demoserver now uses AvalonJettyJervlet instead of JettyJervlet
>
Good stuff.  I got rid of the JettyJervlet wrapper, we are still pre-1.0 
and in the sandbox.

> Next I would like to:
>
> -Make the log redirecting and access logging configurable in 
> AvalonJettyJervlet

+1.  Have a look at what Paul did in the jervlet module.  Could have the 
monitor itself be a component and plug it in using the container, too.

> -Change demoserver (alt create new module) to be more release like for 
> the avalon/loom release already now. This means a new name, and empty 
> documented configuration files, etc. Otherwise very close to demoserver.

Good idea.  Make a loom-jervlet-demo or something.  Then in the future 
we can make others such as plexus-jervlet-demo, pico-jervlet-demo, 
merlin-jervlet-demo.

> -Change the configuration in AvalonJervletJetty to take something like:
> <socketlisteners>
>   <socketlistener>
>      <host/>
>      <port/>
>      <maxthreads/>
>      <minthreads/>
>   </socketlistener>
> </socketlisteners>
> instead of all those in the root level. This way many listeners could 
> be added to a Jetty Server.

+1

> -Set the default port (8080) in AvalonJervletJetty to -1.

+0.  Why?

> -Clean up some parts of the code according to some standard (what 
> should be used btw?)
>
As I play with different projects with different style guidelines, I am 
flexible.  I haven't spent enough time in other spice components' code 
to have a feel for the style guidelines (if there are any) and I don't 
see much on the site.  Maybe follow whatever salt, loggerstore, 
threadpool, or other mature components use.

Also, I think the ${app.home} substitution that was added in 
AbstractJettyJervlet warrants some discussion.  It smells like a 
container service/extension to me, and something that would be used by 
many components that want access to stuff packaged in the sar.  I will 
bring it up on jcontainer-interest.

Thanks again, Johan!

--
Ryan Hoegg
ISIS Networks
http://www.isisnetworks.net



-------------------------------------------------------
This SF.Net email sponsored by: ApacheCon 2003,
16-19 November in Las Vegas. Learn firsthand the latest
developments in Apache, PHP, Perl, XML, Java, MySQL,
WebDAV, and more! http://www.apachecon.com/