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

Hi!

Ryan Hoegg wrote:

>> -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.

I knew someone would object ;) The reason for it was only to make it 
possible to use the original Jetty configuration files without 
modification. Agreed that it brakes IoC. You are also correct about that 
it might produce strange behavior, for example if several components are 
deployed in the same Service Manager etc.

Other possibilities are to include examples of more advanced Jetty 
configurations in the release, or explaing external confs really well in 
the docs. You like writing? ;) Don't worry I'll do it (fingers crossed). 
Anyway, remove it.

>> -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.

Hmm. I was thinking of not spending any time on demos, but start with 
the final release packages already now. Even if they have alfa status. 
For the loom part demoserver is quite close, but its final name can't be 
demoserver :) Soo, why not make loom-jervlet etc. modules already now?

>> -Set the default port (8080) in AvalonJervletJetty to -1.
> 
> +0.  Why?

Ahh, because if you don't specify anything in the AvalonJettyJervlet 
conf it will start listening on port 8080 on all hosts. I don't like 
systems that open stuff by default. It's risky. Note that this is very 
different from having <port>8080</port> and <host>localhost</host> in 
the default configuration. That would be OK, as I see it. -1 means no 
socketlistener btw. If we include the proposed <socketlisteners> conf we 
would have to remove this anyway. Agree?

>> -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.

I'm flexible too. I like to have one for every project though if 
possible. Personally I'm quite fond of the old Avalon coding 
conventions. Don't know if they exist anymore as before.

> Also, I think the ${app.home} substitution that was added in 
> AbstractJettyJervlet warrants some discussion.

The purpose of that is to find the loom/apps/yourapp before knowing 
where in the filesystem the installation exists. Before deploying 
systems all configurations are usually done beforehand as much as 
possible. It helps a lot if you can use some relative root like that. 
Agreed that one could put the webapps dir under e.g. /var/webapps 
instead. Remove it is you don't like it, it sure is a little smelly.


Cheers,

//
Johan



-------------------------------------------------------
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/