Hi Tom,
The changes have been checked in. *toplink.orm.throw.exceptions*
property now covers "ORM file not found" in addition to what it used to
cover. The default value for this property is *changed* from false to
true. As per agreement reached earlier, I had marked issue #1051 as a
FEATURE as opposed to a bug. You also wanted TopLink JPA page to be
updated with the new behavior for this property. I am hoping you shall
take care of this as someone at your end can make the necessary changes.
Thanks,
Sahoo
Tom Ware wrote:
> Hi Marina,
>
> The current behavior is a little different than I thought initially.
>
> 1. If the ORM file cannot be found. We log a Warning. In previous
> discussions about other things, we have agreed that a WARNING is
> fairly severe and that customers seeing warnings should probably
> correct them.
> 2. If there are errors in the ORM file, the
> toplink.orm.throw.exceptions property is used.
>
> This will be the last time I argue sematics with you in this thread,
> but both of these things are designed to be this way and therefore are
> not bugs.
>
> I am still ok with changing the default of this property and even ok
> with adding the "ORM file not found" issue to the list of things this
> property will cover.
>
> In my opinion, we have 1 feature. The feature is to make any error
> in an ORM XML file throw an exception rather than log a message. If
> you want to treat these two things as 2 separate features, that is ok
> with me.
>
> One more comment about release notes. I do not pretend to have any
> say in what goes into the GlassFish release notes, but if I were
> writing them, I would seriously consider noting any behavior that
> could change whether someone application worked from release to
> release in those release notes. I think this change fits that category.
>
> -Tom
>
> Marina Vatkina wrote:
>
>> Hi Tom,
>>
>> With the current bug (it is a bug) even if you try to turn exceptions
>> on,
>> the code will never reach that block. When Sahoo fixes the bug, the new
>> feature will enable an exception by default.
>>
>> I agree with #1 and #3.
>>
>> thanks,
>> -marina
>>
>> Tom Ware wrote:
>>
>>
>>> 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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>