-
Notifications
You must be signed in to change notification settings - Fork 467
Introduce multiple datasources #4812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
df64e6c
bb3cb70
b88e82f
14b5d27
2fab4d2
c5f6829
b9f7d31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,6 @@ | |
|
|
||
| import io.smallrye.common.annotation.Identifier; | ||
| import jakarta.enterprise.context.ApplicationScoped; | ||
| import jakarta.enterprise.inject.Instance; | ||
| import jakarta.enterprise.inject.Produces; | ||
| import jakarta.inject.Inject; | ||
| import java.sql.SQLException; | ||
| import java.time.Clock; | ||
|
|
@@ -32,7 +30,6 @@ | |
| import java.util.Optional; | ||
| import java.util.Set; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import javax.sql.DataSource; | ||
| import org.apache.polaris.core.PolarisCallContext; | ||
| import org.apache.polaris.core.PolarisDiagnostics; | ||
| import org.apache.polaris.core.config.BehaviorChangeConfiguration; | ||
|
|
@@ -93,13 +90,6 @@ public class JdbcMetaStoreManagerFactory implements MetaStoreManagerFactory { | |
|
|
||
| protected JdbcMetaStoreManagerFactory() {} | ||
|
|
||
| @Produces | ||
| @ApplicationScoped | ||
| static DatasourceOperations produceDatasourceOperations( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably for my information, why do we move it? Should this be part of JDBC module for better modularization?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had a strong requirement for this move: the new producer method now needs to access |
||
| Instance<DataSource> dataSource, RelationalJdbcConfiguration relationalJdbcConfiguration) { | ||
| return new DatasourceOperations(dataSource.get(), relationalJdbcConfiguration); | ||
| } | ||
|
|
||
| protected PrincipalSecretsGenerator secretsGenerator( | ||
| String realmId, @Nullable RootCredentialsSet rootCredentialsSet) { | ||
| if (rootCredentialsSet != null) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,18 +21,21 @@ | |
| import java.util.Optional; | ||
|
|
||
| public interface RelationalJdbcConfiguration { | ||
| // max retries before giving up | ||
| /** The maximum number of retries before giving up the operation. */ | ||
| Optional<Integer> maxRetries(); | ||
|
|
||
| // max retry duration | ||
| /** The maximum retry duration in milliseconds. */ | ||
| Optional<Long> maxDurationInMs(); | ||
|
|
||
| // initial delay | ||
| /** The initial retry delay. */ | ||
| Optional<Long> initialDelayInMs(); | ||
|
|
||
| /** | ||
| * Explicitly configured database type. If not specified, the database type will be inferred from | ||
| * the JDBC connection metadata. Supported values: "postgresql", "cockroachdb", "h2" | ||
| */ | ||
| Optional<String> databaseType(); | ||
|
|
||
| /** The datasource name to use. Required. */ | ||
| String dataSource(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: the datasource name is distinct from the database type. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,12 @@ quarkus.container-image.registry=docker.io | |
| quarkus.container-image.group=apache | ||
| quarkus.container-image.name=polaris-admin-tool | ||
| quarkus.container-image.additional-tags=latest | ||
| quarkus.datasource.db-kind=postgresql | ||
|
|
||
| # Named datasources for relational persistence backends. | ||
| # Polaris ships with many built-in datasources; by using named datasources, users can configure | ||
| # at runtime which datasources to use for relational persistence backends. | ||
| quarkus.datasource.postgresql.db-kind=postgresql | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The admin module does not ship H2, at least for now. We may want to include H2 later. There are pros and cons:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure H2 helps - at least not as long it's only in-memory. |
||
|
|
||
| # if set to true it will try to start localstack at build and run time for the local environment | ||
| # https://docs.quarkiverse.io/quarkus-amazon-services/dev/amazon-rds.html#_configuration_reference for more details | ||
| quarkus.rds.devservices.enabled=false | ||
|
|
@@ -41,12 +46,13 @@ quarkus.mongodb.devservices.enabled=false | |
| # ---- Runtime Configuration ---- | ||
| # Below are default values for properties that can be changed in runtime. | ||
|
|
||
| # Available types: | ||
| # Polaris persistence type. Available types: | ||
| # - in-memory - InMemoryPolarisMetaStoreManagerFactory | ||
| # - in-memory-atomic - InMemoryAtomicOperationMetaStoreManagerFactory | ||
| # - relational-jdbc - JdbcMetaStoreManagerFactory | ||
| # - nosql - NoSQL persistence backend, define the backend type via 'polaris.persistence.nosql.backend' | ||
| # - nosql (beta) - NoSQL persistence backend, define the backend type via 'polaris.persistence.nosql.backend' | ||
| # - relational-jdbc - JDBC persistence backend, define the JDBC datasource via 'polaris.persistence.relational.jdbc.datasource' and 'quarkus.datasource.<datasource-name>.*' | ||
| polaris.persistence.type=relational-jdbc | ||
|
|
||
| # Database backend for 'nosql' persistence-type | ||
| # Available backends: | ||
| # - InMemory - for testing purposes | ||
|
|
@@ -55,6 +61,21 @@ polaris.persistence.type=relational-jdbc | |
| # See https://quarkus.io/guides/mongodb#configuration-reference for details about these configurations. | ||
| #polaris.persistence.nosql.backend=InMemory | ||
|
|
||
| # Datasource to activate for the 'relational-jdbc' persistence type. | ||
| # Built-in supported datasources: | ||
| # - postgresql - activates the PostgreSQL datasource (default) | ||
| # Configure the necessary PostgreSQL properties starting with 'quarkus.datasource.postgresql.' in this file (see below). | ||
| # See https://quarkus.io/guides/datasource#configuration-reference for details about these configurations. | ||
| # Please do NOT set quarkus.datasource.active manually, Polaris will set it automatically based on | ||
| # this property. | ||
| polaris.persistence.relational.jdbc.datasource=postgresql | ||
|
|
||
| # Postgres datasource configuration: | ||
| quarkus.datasource.postgresql.devservices.enabled=false | ||
| #quarkus.datasource.postgresql.jdbc.url=jdbc:postgresql://localhost:5432/my_database | ||
| #quarkus.datasource.postgresql.username=<your username> | ||
| #quarkus.datasource.postgresql.password=<your password> | ||
|
|
||
| ## MongoDB version store specific configuration | ||
| #quarkus.mongodb.database=polaris | ||
| #quarkus.mongodb.metrics.enabled=true | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a reasonable notice to users, IMHO... However, given that the 1.6.0 release is supposed to happen pretty soon, should we defer merging this PR until after 1.6.0 is branched?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's reasonable. We can defer this to 1.7.0, which would then include the complete switch to JDBC+H2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that 1.6 is branches, I guess this is no longer a concern ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed: +1 to merge to
mainnow.