You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2006/03/10 16:19:14 UTC

[dbcp] BZ 38073 - JNDI problems with SharedPoolDataSource

Any comments on this?
http://issues.apache.org/bugzilla/show_bug.cgi?id=38073

As I see it, there are two decent alternatives here.  The one I first
suggested in the report is too ugly.

(i) Make the (incorrectly) implemenented getReference method in
InstanceKeyDataSource throw, as described in the last comment on the
report, and override with correct impls in the concrete subclasses.

(ii) Make the (minor) backward-incompatible change to make the
offending method in InstanceKeyDataSource abstract, as suggested in
the original report, again providing correct impls in the subclasses.

I think (ii) is cleaner (what should have been done in the first
place) but is technically a backward incompatible change.  Users who
have subclassed will be forced to implement.

Interested parties, please weigh in with opinions or better ideas
about how to resolve this.  Also scream now if adding test
dependencies on the (ibiblio published) tomcat naming jars is not
acceptable.  Thanks!

Phil

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


Re: [dbcp] BZ 38073 - JNDI problems with SharedPoolDataSource

Posted by Phil Steitz <ph...@gmail.com>.
On 3/10/06, Sandy McArthur <sa...@apache.org> wrote:
> On 3/10/06, Phil Steitz <ph...@gmail.com> wrote:
> > Any comments on this?
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=38073
> >
> > As I see it, there are two decent alternatives here.  The one I first
> > suggested in the report is too ugly.
> >
> > (i) Make the (incorrectly) implemenented getReference method in
> > InstanceKeyDataSource throw, as described in the last comment on the
> > report, and override with correct impls in the concrete subclasses.
> >
> > (ii) Make the (minor) backward-incompatible change to make the
> > offending method in InstanceKeyDataSource abstract, as suggested in
> > the original report, again providing correct impls in the subclasses.
> >
> > I think (ii) is cleaner (what should have been done in the first
> > place) but is technically a backward incompatible change.  Users who
> > have subclassed will be forced to implement.
> >
> > Interested parties, please weigh in with opinions or better ideas
> > about how to resolve this.
>
> (Standard disclaimer about not being familiar with Dbcp yet.)
>
> For a bug fix release it seems to me that you should override
> getReference() in PerUserPoolDataSource and SharedPoolDataSource. And
> document in InstanceKeyDataSource.getReference() that it will be going
> away in the next major release.
>
> Also, it seems to me that changing
> InstanceKeyDataSource.getReference() for the ref var to:
>        Reference ref = new Reference(getClass().getName(),
>            getClass().getName() + "Factory", null);
>
> would work better. It's fragile but I don't think it would break anything new.
>
> > Also scream now if adding test
> > dependencies on the (ibiblio published) tomcat naming jars is not
> > acceptable.
>
> For building and testing only right? Not for deployment of Dbcp?

Yes, just a test dependency.  Not required at all for dist or runtime.

Phil
>
> --
> Sandy McArthur
>
> "He who dares not offend cannot be honest."
> - Thomas Paine
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>

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


Re: [dbcp] BZ 38073 - JNDI problems with SharedPoolDataSource

Posted by Sandy McArthur <sa...@apache.org>.
On 3/10/06, Phil Steitz <ph...@gmail.com> wrote:
> Any comments on this?
> http://issues.apache.org/bugzilla/show_bug.cgi?id=38073
>
> As I see it, there are two decent alternatives here.  The one I first
> suggested in the report is too ugly.
>
> (i) Make the (incorrectly) implemenented getReference method in
> InstanceKeyDataSource throw, as described in the last comment on the
> report, and override with correct impls in the concrete subclasses.
>
> (ii) Make the (minor) backward-incompatible change to make the
> offending method in InstanceKeyDataSource abstract, as suggested in
> the original report, again providing correct impls in the subclasses.
>
> I think (ii) is cleaner (what should have been done in the first
> place) but is technically a backward incompatible change.  Users who
> have subclassed will be forced to implement.
>
> Interested parties, please weigh in with opinions or better ideas
> about how to resolve this.

(Standard disclaimer about not being familiar with Dbcp yet.)

For a bug fix release it seems to me that you should override
getReference() in PerUserPoolDataSource and SharedPoolDataSource. And
document in InstanceKeyDataSource.getReference() that it will be going
away in the next major release.

Also, it seems to me that changing
InstanceKeyDataSource.getReference() for the ref var to:
        Reference ref = new Reference(getClass().getName(),
            getClass().getName() + "Factory", null);

would work better. It's fragile but I don't think it would break anything new.

> Also scream now if adding test
> dependencies on the (ibiblio published) tomcat naming jars is not
> acceptable.

For building and testing only right? Not for deployment of Dbcp?

--
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

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


Re: [dbcp] BZ 38073 - JNDI problems with SharedPoolDataSource

Posted by Sandy McArthur <sa...@apache.org>.
I've created a patch with what I think is the best fix for now:
http://issues.apache.org/bugzilla/show_bug.cgi?id=38073#c8

On 3/10/06, Noel J. Bergman <no...@devtech.com> wrote:
> Phil Steitz wrote:
>
> > Any comments on this?
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=38073
>
> I'd go with the cleaner solution, and document the possible problem in the
> release notes for anyone whom it might impact.

--
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

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


RE: [dbcp] BZ 38073 - JNDI problems with SharedPoolDataSource

Posted by "Noel J. Bergman" <no...@devtech.com>.
Phil Steitz wrote:

> Any comments on this?
> http://issues.apache.org/bugzilla/show_bug.cgi?id=38073

I'd go with the cleaner solution, and document the possible problem in the
release notes for anyone whom it might impact.

	--- Noel


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