commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [dbcp] Changes in R734470, 734481 to replace SQLNestedException
Date Fri, 23 Jan 2009 00:05:17 GMT
On 22/01/2009, Niall Pemberton <niall.pemberton@gmail.com> wrote:
> 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

OK, done.

I kept the other change from 734470, which was the removal of an
impossible null check.

> Niall
>
>
>  > 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


Mime
View raw message