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 14:25:52 -0400

Hi Marina,

  For some reason, I can't get to the GF web pages right now. I'll try
again later.

  Let's do #1 and #3

  I have started the process for #3 and it should be possible to
complete it by the time Sahoo finishes his changes.

  Are we ok with changing the current bug to a Feature request and
updating the subject line? Or do we want to close the current issue and
point to a new feature request?

-Tom

Marina Vatkina wrote:

>Hi Tom,
>
>Fair enough. We have several options:
>
>1. File a feature request and mark it as fixed.
>2. Add a note to GF persistence page.
>3. Add a note to TopLink JPA page where you list this option.
>
>What would be your preference? I don't think it's a candidate for the release
>notes.
>
>thanks,
>-marina
>
>Tom Ware wrote:
>
>
>>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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>