[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14280404#comment-14280404
]
Phil Steitz commented on DBCP-423:
--
I get your point, Thomas, but I still like the BDS approach better. I think it
makes sense to wrap and rethrow because the wrapping is appropriate to the
context. The exception happened while closing the datasource, so at this API
boundary it makes sense for it to be an SQLException, regardless of root cause.
Sure, what is wrapped will almost certainly be an SQLException already, but
the wrapping ensures this and adds the message that it happened while closing
the datasource. I might actually have done the same thing on the unchecked
path for the same reason.
Are you OK if I change the patch to do what the BDS close does?
Any other opinions on this?
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Attachments: DBCP-423.patch
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14280430#comment-14280430
]
Thomas Neidhart commented on DBCP-423:
--
Maybe I was not clear, but I meant to do something like this:
{code}
try {
_pool.close();
} catch(SQLException e) {
throw e;
} catch(RuntimeException e) {
throw e;
} catch(Exception e) {
throw new SQLException(blabla, e);
}
{code}
previously, we *always* wrapped the caught exception into an SQLException,
which does not make much sense imho.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Attachments: DBCP-423.patch
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14280434#comment-14280434
]
Sebb commented on DBCP-423:
---
+1 to Phil's approach.
Wrapping shows that the method has properly dealt with the Exception.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Attachments: DBCP-423.patch
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14280450#comment-14280450
]
Sebb commented on DBCP-423:
---
There are two aspects to this: if the code sometimes wraps and sometimes
doesn't it's not immediately clear that the unwrapped Exceptions were properly
dealt with.
It's potentially harder for higher level code to unravel as well.
The other aspect is that the wrapper Exception should have a message that
describes how the original exception affected the method and what has been done
about it, i.e. it needs to add useful context.
An analogy might be EOF or IOE when reading a file. By itself it means little,
so the caller needs to wrap it e.g. to say what it was trying to do when the
error occurred (e.g. read the trailer). The caller's caller may in turn need to
wrap again to give context to the specific file being processed. Etc.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Attachments: DBCP-423.patch
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14280513#comment-14280513
]
Thomas Neidhart commented on DBCP-423:
--
I am fine with both approaches, but would like to see it applied consistantly
throughout the library. The code snippet was taken from PoolableConnection to
show how some parts of the code handle situations like this.
@properly dealt with: in the case of close() there is no exception handling,
anything that happens is just thrown further, sometimes wrapped, sometimes not,
so I don't think this argument applies here.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Attachments: DBCP-423.patch
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14280654#comment-14280654
]
Phil Steitz commented on DBCP-423:
--
Thomas - I am sorry I did not understand what you were proposing. I like the
PC approach and would be happy implementing that instead of the BDS approach
here. I would also be open to modifying the RuntimeException catch to wrap and
add a context message.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Attachments: DBCP-423.patch
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14277809#comment-14277809
]
Thomas Neidhart commented on DBCP-423:
--
It is unfortunate that we can not touch the exception behavior but this should
be done in another major release imho.
On a second thought, the approach in BDS should be further improved as it now
catches a generic Exception and wraps it into a SQLException. If the pool will
throw an exception in its close method it will most likely be already a
SQLException (coming from the PoolableConnection implementation for example).
Thus we should not wrap a SQLException inside a SQLException but propagate the
SQLException. The same is already done in the PoolableConnection.close() method.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Attachments: DBCP-423.patch
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14273014#comment-14273014
]
Phil Steitz commented on DBCP-423:
--
Patch looks good to me. I agree it would be better to harmonize the exception
behavior, but I am not sure we can fix this in a minor release, as it could
break some PerUserPoolDS clients, since that implementation actually swallows.
SharedPoolDataSource propagates. I actually like the BDS approach the best,
but I am OK with straight propagation.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Attachments: DBCP-423.patch
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14262815#comment-14262815
]
Christian Schneider commented on DBCP-423:
--
I think if I open a DataSource in try with resources then I would also expect
it to be closed at the end. So I do not see a big problem with that one.
Can you imagine an example where somone creates a DataSource in try with
resources but does not want it to be closed?
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14262819#comment-14262819
]
Mark Thomas commented on DBCP-423:
--
+1 to AutoClosable. We can call out the change in behaviour in the release
notes. If this was a bug fix release (i.e. 2.0.1) then I'd be concerned but I
think this is OK in a 2.1 release.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14262958#comment-14262958
]
Phil Steitz commented on DBCP-423:
--
Christian - I can't honestly. Would have to be sloppy code; but I was worried
about the behavior change. I guess its OK to doc in release notes.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14262589#comment-14262589
]
Sebb commented on DBCP-423:
---
Closeable classes trigger IDE warnings about resource leaks if the code does
not call close().
If close() does not have to be called for PDS, this would be a nuisance, but if
PDS should always be closed, then it might be a useful check.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14262612#comment-14262612
]
Phil Steitz commented on DBCP-423:
--
Sebb - I guess that sort of confirms the smelliness. I can see how IDEs could
meaningfully / helpfully point out when io streams are opened but not closed.
A datasource is not a stream and the thread that opens it may not be the thread
the closes it. What seems funny to me is to implement the java.io,Closeable
interface on a java.sql resource. Put in another way, if this is natural, why
doesn't java.sql.Connection implement it? I think this is really something for
the client code to handle. I am +1 for adding a good close() to PDS though.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14262652#comment-14262652
]
Christian Schneider commented on DBCP-423:
--
For me having a close in PoolingDataSource would not help. I close the
DataSource in pax-jdbc-config. There only DataSource is known and the module
should have no dependency on dbcp. This is why I used Closeable as it is
present in the jdk. So basically what I need is that the PoolingDataSource impl
implements a standard java interface with a close method.
Honestly I think that not implementing something like Closeable in many Java
APIs is the real smell .. together with Closeable.close throwing a checked
exception which totally sucks from a client perspective. Still Closeable is the
only present interface to mark that something can be closed. Btw. If you look
at newer Java APIs like jms 2.0 you see that they extend AutoClosable. So I
think the spec designers understood that they left out an important part by not
having a universal closing facility and corrected this.
So of course extending Autoclosable would be the even better thing but it is
Java 7 only so I do not know if it is ok for dbcp.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14262695#comment-14262695
]
Phil Steitz commented on DBCP-423:
--
DBCP 2 requires Java 7+, so this should work. That interface looks like
exactly what we need. Unless I hear objections, or someone beats me to it with
a patch or commit, I will implement close properly in PDS and have both it and
BDS implement Autocloseable.
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (DBCP-423) PoolingDataSource should implement Closeable
[
https://issues.apache.org/jira/browse/DBCP-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14262378#comment-14262378
]
Phil Steitz commented on DBCP-423:
--
It seems a little smelly to me to implement Closeable from java.io here. I
would be OK adding a close method to PoolingDataSource that properly handles
edge cases, etc, like the close in BasicDataSource does. Would that be
sufficient? Any other thoughts on this?
PoolingDataSource should implement Closeable
Key: DBCP-423
URL: https://issues.apache.org/jira/browse/DBCP-423
Project: Commons Dbcp
Issue Type: Improvement
Affects Versions: 2.0
Reporter: Christian Schneider
Fix For: 2.1
Currently PoolingDataSource only implements DataSource.
I have the following case in ops4j pax-jdbc. I offer a DataSourceFactory in
one bundle that can create a pooling DataSource.
Then in another bundle I create DataSources based on config in
ConfigurationAdmin. So when the config appears I create the DataSource, when
the config goes away I have to destroy it.
It is important to correctly dispose the DataSource as the pool has to be
closed. As I can not depend on dbcp in the bundle that destroys the
DataSource I currently have no simple way to destroy the DataSource.
This is where I create the DataSource:
https://github.com/ops4j/org.ops4j.pax.jdbc/blob/master/pax-jdbc-pool/src/main/java/org/ops4j/pax/jdbc/pool/impl/PooledDataSourceFactory.java
I was able to solve it by a kind of hack. I extended the PoolingDataSource
with a class that supports Closeable. So from the other bundle I could check
if the object implements Closeable and call close.
It would be a lot easier if PoolingDataSource would implement Closeable like
I did. The same is true for GenericObjectPool and the other pools. If they
would implement Closeable then handling them would be a lot easier.
They already support a close method so it would be a small change.
public class CloseablePoolingDataSourceC extends Connection extends
PoolingDataSourceC implements Closeable {
public CloseablePoolingDataSource(ObjectPoolC pool) {
super(pool);
}
@Override
public void close() throws IOException {
getPool().close();
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
