commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [dbcp] Changes in R734470, 734481 to replace SQLNestedException
Date Wed, 21 Jan 2009 11:16:06 GMT
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.

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


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


Mime
View raw message