dimas-b commented on code in PR #4984:
URL: https://github.com/apache/polaris/pull/4984#discussion_r3532357359
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -69,14 +69,23 @@ public class DatasourceOperations {
private final DataSource datasource;
private final RelationalJdbcConfiguration relationalJdbcConfiguration;
+ private final boolean closeDataSourceOnClose;
private final DatabaseType databaseType;
private static final Random random = new Random();
public DatasourceOperations(
DataSource datasource, RelationalJdbcConfiguration
relationalJdbcConfiguration) {
+ this(datasource, relationalJdbcConfiguration, false);
+ }
+
+ DatasourceOperations(
+ DataSource datasource,
+ RelationalJdbcConfiguration relationalJdbcConfiguration,
+ boolean closeDataSourceOnClose) {
Review Comment:
`closeDataSourceOnClose` is awkward to read. How about
`assumeDataSourceOwnership`?
##########
site/content/in-dev/unreleased/metastores/relational-jdbc.md:
##########
@@ -22,12 +22,18 @@ type: docs
weight: 100
---
-This implementation leverages Quarkus for datasource management and supports
configuration through
-environment variables or JVM -D flags at startup. For more information, refer
to the [Quarkus configuration
reference](https://quarkus.io/guides/config-reference#env-file).
+The Relational JDBC metastore supports PostgreSQL, CockroachDB, and H2. H2 is
intended for testing
+and development only.
-We have 2 options for configuring the persistence backend:
+Polaris can use either a Quarkus-managed datasource or a Polaris-managed JDBC
connection pool. When
+`polaris.persistence.relational.jdbc.jdbc-url` is set, Polaris creates the
JDBC pool directly;
Review Comment:
The only advantage of doing this, that I can see, realizes when Polaris is
able to load the JDBC driver dynamically. However, I could not find any
provisions for dynamic loading in this PR. Did I miss that? 🤔
If repackaging at the binary level is assumed, a Quakus build can be
performed at that stage to make the DataSource available via Quarkus-backed CDI.
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -98,6 +107,21 @@ DatabaseType getDatabaseType() {
return databaseType;
}
+ boolean ownsDataSource() {
+ return closeDataSourceOnClose;
+ }
+
+ @Override
+ public void close() {
Review Comment:
When is this `close()` method called?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
