geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Jencks <david_jen...@yahoo.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:44:00 GMT

On Jun 19, 2008, at 10:26 AM, Kevan Miller wrote:

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

Not me :-), at least not yet.  Unfortunately I don't remember what  
eventually creates this file.  I thought I once got rid of this error  
message by making sure the file was created at the appropriate time.

My original idea was that since a particular config- 
substitutions.properties type file was defined by a server instance  
gbean we shouldn't create any such files unless we knew how they  
should be named, from a server-instance gbean.

I'm in the middle of some other stuff and don't want to change gears  
today.... I would start investigating by figuring out what does  
eventually create the files and why that isn't run first during  
assembly.

>
> 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...
>
instructions would be good.  I think there's code in or near the  
plugin gbean that writes it out.

thanks
david jencks

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