persistence@glassfish.java.net

Re: issue #1051: throw exception if mapping file is not found

From: Tom Ware <tom.ware_at_oracle.com>
Date: Mon, 25 Sep 2006 13:50:45 -0400

Hi Marina,

  First of all, saying we are ignoring the issue is conpletely false. A
warning message is logged. In fact, having the exception thrown is even
a configuration option.

  We have now chosen to change this default value for this option.

  The fact is, that whether or not you personally like the initial
design choice, that is what the current product does and there could be
customers that are able to deploy an application that runs successfully
as a result of that choice. With this change, their applications will
potentially no longer deploy. I think it is perfectly reasonable to
publish this kind of information somewhere.

  My question is simply: In this open source environment, how do we
publish changes to defaults from build to build when they are NOT bugs,
but simply new design choices.

-Tom


Marina Vatkina wrote:

>Hi Tom,
>
>Are you concerned that throwing an exception when we should, instead of ignoring
>it will as we do today, will cause some users to clean up their applications?
>
>thanks,
>-marina
>
>Tom Ware wrote:
>
>
>>Hi Sahoo,
>>
>> I am, in principal, ok with changing the default.
>>
>> Here is a more general question. If we decide to change a default like
>>this (that could affect backward compatibility for some users). How do
>>we notify them? To me, just closing the issue is not really adequate in
>>the case were we are making a conscious choice to change behavior rather
>>than fixing something that is clearly a bug.
>>
>>-Tom
>>
>>Sanjeeb Kumar Sahoo wrote:
>>
>>
>>
>>>Hi Tom,
>>>
>>>Tom Ware wrote:
>>>
>>>
>>>
>>>
>>>>Hi Sahoo,
>>>>
>>>>Choosing to just warn for an orm.xml file that does not exist was a
>>>>design choice. I think both the current behavior and the behavior
>>>>you are suggesting are quite reasonable.
>>>>We initially chose to only send a warning to allow developers to
>>>>continue to work with their persistence unit even if there were some
>>>>errors in some of the files. Changing the value
>>>>toplink.orm.throw.exceptions should allow the exceptions to actually
>>>>be thrown. Is simply documenting this property adequate to fix this
>>>>issue?
>>>>
>>>>
>>>>
>>>No. I think, we should change the default (see further explanation
>>>given below) in addition to documenting the property.
>>>
>>>
>>>
>>>
>>>>Is there any particular reason why we are choosing to change this
>>>>default?
>>>>
>>>>
>>>>
>>>>
>>>More often than not, an application will run into some kind of trouble
>>>if a mapping file is not processed. The submitter of issue #1051 also
>>>wants us to throw exception. I support him, because we both wasted 15
>>>to 20 minutes before we found out the typo in mapping file name. Given
>>>the number of messages that get logged, it is very easy to miss a
>>>warning. Why would a (real) developer ever want to specify a
>>>non-existent mapping file? So I think, throwing an exception is a more
>>>developer friendly choice.
>>>
>>>So, let me know if I can change the default behavior or not. I would
>>>also like to know what others think.
>>>
>>>Thanks,
>>>Sahoo
>>>
>>>
>>>
>>>
>>>>-Tom
>>>>
>>>>Sanjeeb Kumar Sahoo wrote:
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>Hi Tom,
>>>>>
>>>>>I am working on
>>>>>https://glassfish.dev.java.net/issues/show_bug.cgi?id=1051. The
>>>>>current behavior is to log a message when mapping file does not
>>>>>exist. I am planning to change it to throw an exception by default,
>>>>>but I see some entity-persistence-tests failing as they specifically
>>>>>test for the current behavior. The persistence.xml has such text:
>>>>><!-- this is here for testing purposes - ensures a persistence unit
>>>>>can load even with a file that does not exist. Please do not add an
>>>>>xml file called: META-INF/non-existant-orm-file.xml -->
>>>>>
>>>>><mapping-file>META-INF/non-existant-orm-file.xml</mapping-file>
>>>>>
>>>>>Is there a specific reason behind the current behavior? Can we
>>>>>change it?
>>>>>
>>>>>Also, what's this property called toplink.orm.throw.exceptions? I
>>>>>don't find any documentation for this property. It is used in
>>>>>current code.
>>>>>
>>>>>Thanks,
>>>>>Sahoo
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>