commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Niall Pemberton <niall.pember...@gmail.com>
Subject Re: [dbcp] Changes in R734470, 734481 to replace SQLNestedException
Date Thu, 22 Jan 2009 21:11:45 GMT
On Wed, Jan 21, 2009 at 11:16 AM, Phil Steitz <phil.steitz@gmail.com> wrote:
> sebb wrote:
>>
>> On 20/01/2009, Phil Steitz <phil.steitz@gmail.com> wrote:
>>
>>>
>>> I would like to ask that these changes be reverted.   Deprecated classes
>>> and
>>> client code breakage that depends on them can only happen in major (x.0)
>>> releases.  Unless I am missing something subtle, these changes will break
>>> client code that specializes the cast on caught exceptions and tries to
>>> unpack the nested root cause exception.
>>>
>>
>> Throwable.getCause() was added in 1.4, and DBCP now targets 1.5+
>> (according to the Maven build files) so there won't be a problem
>> calling getCause(), however the cast to SQLNestedException would fail.
>>
>>
>>>
>>> Since this is a .y release, we
>>> should not make this change at this point.
>>>
>>
>> OK, I was a bit hasty (though I did ask first, I did not wait long
>> enough).
>>
>
> Sorry I took so long to review.
>>
>> Rather than remove the new code, how about changing:
>>
>> throw (SQLException) new SQLException(message).initCause(e);
>>
>> into:
>>
>> throw (SQLException) new SQLNestedException(message).setCause(e);
>>
>> The SQLNestedException class would need to be extended to add a
>> (String) constructor and a setCause() method which copies the relevant
>> code from the existing constructor.
>>
>> That is easy to do now and easy to undo when the time comes to remove
>> the deprecated class. I think it maintains full compatibility as well.
>>
>> [If overriding Throwable.initCause() is allowed, then the change could
>> be simplified slightly]
>>
>> WDYT?
>>
>
> Honestly, I think the best solution is to revert the change.  When, if ever,
> DBCP sees a 2.0, we can clean remove SQLNestedException and clean up
> systematically.  I don't see the risk of other unanticipated impacts as
> being worth the simplification in cleanup later.  We would also have to add
> test cases.  So my vote is for less work  and breakage risk now, which means
> revert the change.

+1
Niall

> Phil
>>>
>>>  Phil

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message