geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevan Miller <kevan.mil...@gmail.com>
Subject Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
Date Thu, 19 Jun 2008 17:26:29 GMT

On Jun 19, 2008, at 12:27 PM, Lin Sun wrote:

> I think the current exception is rather confusion when a user tries to
> assemble a server from admin console.   He will see an error message
> saying the file was not found in the server's console which makes him
> to think the created custom assembly zip file is incorrect.
>
> How about we change to - when the file is not found, log it as a
> warning or info?   For other exceptions, log it as an error as
> currently the code is doing.
>
> Lin
>
> On Thu, Jun 19, 2008 at 11:59 AM, David Jencks  
> <david_jencks@yahoo.com> wrote:
>> I agree with Jarek, -1 on this change.  The warnings during the  
>> build are
>> fairly harmless.  Ideally we could figure out how to make the file  
>> be there
>> before we try to access it during server assembly.  I'd be be ok with
>> logging an error but slightly prefer the current exception unless  
>> we find
>> there is no good way to create the file before it is needed.

They look like "errors" to me ;-P. And disagree that they are  
harmless. As described in the jira, this is not just during a  
"build" (i.e. while running mvn). It happens when exporting an  
assembly from a running server. I think users would justifiably think  
that something has gone wrong.

That said, I also agree that it's better to inform users when we don't  
find a config-substitutions.properties file.

The appropriate solution, IMO, is to include a config- 
substitutions.properties file in geronimo-boilerplate-minimal.jar. I  
think everyone will be happy, then.

Just tried this (added a zero-length geronimo-boilerplate-minimal/src/ 
main/underlay/var/config/config-substitutions.properties file). It  
causes a problem in the geronimo-framework assembly -- we don't write  
out any properties and framework server start-up fails. We write out  
properties for in the tomcat/jetty assemblies, but they don't contain  
framework properties. I'd guess that we can fix this problem, pretty  
easily...

While we're at it, perhaps we can add a comment to config- 
substitutions.properties that indicates it's usage? The README.txt  
talks about it, but useful to have a little doc in the file itself...

--kevan


>>
>>
>> thanks
>> david jencks
>>
>> On Jun 19, 2008, at 8:49 AM, Jarek Gawor wrote:
>>
>>> Lin,
>>>
>>> I wonder if we should instead log a warning when the file was
>>> specified but not found. With this change and in most cases where
>>> LocalAttributeManager is used, the user will have no idea that the
>>> file was not read (and later might result in weird exceptions as the
>>> variables in config.xml did not get resolved). Or maybe we need to  
>>> do
>>> something special for the "assemble the server" case since these
>>> FilleNotFound errors are only visible there.
>>>
>>> Jarek
>>>
>>> On Thu, Jun 19, 2008 at 11:27 AM,  <linsun@apache.org> wrote:
>>>>
>>>> Author: linsun
>>>> Date: Thu Jun 19 08:27:35 2008
>>>> New Revision: 669506
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=669506&view=rev
>>>> Log:
>>>> GERONIMO-3971 - Error message during assembling a server
>>>>
>>>> Modified:
>>>>
>>>> geronimo/server/trunk/framework/modules/geronimo-system/src/main/ 
>>>> java/org/apache/geronimo/system/configuration/ 
>>>> LocalAttributeManager.java
>>>>
>>>> Modified:
>>>> geronimo/server/trunk/framework/modules/geronimo-system/src/main/ 
>>>> java/org/apache/geronimo/system/configuration/ 
>>>> LocalAttributeManager.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java?rev=669506&r1=669505&r2=669506&view=diff
>>>>
>>>> = 
>>>> = 
>>>> = 
>>>> = 
>>>> = 
>>>> = 
>>>> = 
>>>> = 
>>>> = 
>>>> = 
>>>> = 
>>>> ===================================================================
>>>> ---
>>>> geronimo/server/trunk/framework/modules/geronimo-system/src/main/ 
>>>> java/org/apache/geronimo/system/configuration/ 
>>>> LocalAttributeManager.java
>>>> (original)
>>>> +++
>>>> geronimo/server/trunk/framework/modules/geronimo-system/src/main/ 
>>>> java/org/apache/geronimo/system/configuration/ 
>>>> LocalAttributeManager.java
>>>> Thu Jun 19 08:27:35 2008
>>>> @@ -615,7 +615,7 @@
>>>>
>>>>  private static Properties loadConfigSubstitutions(File
>>>> configSubstitutionsFile) {
>>>>      Properties properties = new Properties();
>>>> -        if (configSubstitutionsFile != null) {
>>>> +        if (configSubstitutionsFile != null &&
>>>> configSubstitutionsFile.exists()) {
>>>>          try {
>>>>              FileInputStream in = new
>>>> FileInputStream(configSubstitutionsFile);
>>>>              try {
>>>>
>>>>
>>>>
>>
>>


Mime
View raw message