You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Jarek Gawor <jg...@gmail.com> on 2008/06/19 17:49:04 UTC
Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
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, <li...@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 {
>
>
>
Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
Posted by Kevan Miller <ke...@gmail.com>.
On Jun 19, 2008, at 1:44 PM, David Jencks wrote:
>
> 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.
Sounds good. As long as we have this specific problem resolved, I'll
prolly be happy with the specifics of the fix.
--kevan
Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
Posted by David Jencks <da...@yahoo.com>.
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, <li...@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 {
>>>>>
>>>>>
>>>>>
>>>
>>>
>
Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
Posted by Lin Sun <li...@gmail.com>.
I agree I've been always wondering if the assembly zip file is wrong! :-)
I can change the behavior to warn the users when we don't find the
config-substitutions.properties file before David has time to look
into a better solution. I don't think adding the
config-substitutions.properties file to
geronimo-boilerplate-minimal.jar would work, as the filenotfound
exception was thrown when the PluginInstallerGBean is initialized and
at that time the system doesn't have any files under
var\temp\assembly\var\config yet.
I'll also try add some instructions in the beginning of the
config-substitutions.properties file. looks like it can be done in
the storeConfigSubstitutions method of LocalAttributeManager.
thanks, Lin
On Thu, Jun 19, 2008 at 1:26 PM, Kevan Miller <ke...@gmail.com> wrote:
> 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, <li...@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 {
>
>
>
>
>
>
>
Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
Posted by Kevan Miller <ke...@gmail.com>.
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
> <da...@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, <li...@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 {
>>>>
>>>>
>>>>
>>
>>
Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
Posted by Lin Sun <li...@gmail.com>.
BTW, the warnings are not only during the build, otherwise I would not
want to change it. :)
In the custom server assmebly case, depends on whatever module the
user selects to assembly the server, the config substitution file may
or may not need to be created. Thus it is normal when the code
reports an java.io.FileNotFoundException. The only issue is that the
code reports it as an error.
Lin
>
> On Thu, Jun 19, 2008 at 11:59 AM, David Jencks <da...@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.
>>
>> 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, <li...@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 {
>>>>
>>>>
>>>>
>>
>>
>
Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
Posted by Lin Sun <li...@gmail.com>.
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 <da...@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.
>
> 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, <li...@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 {
>>>
>>>
>>>
>
>
Re: svn commit: r669506 - /geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/LocalAttributeManager.java
Posted by David Jencks <da...@yahoo.com>.
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.
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, <li...@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 {
>>
>>
>>