feat: Add Global Datasource configuration application for data-source-properties #1648
Conversation
|
|
|
Hi @graemerocher , I was wondering if it would be possible to get this merged in at some point? not sure who else to ping to get a review |
| * | ||
| * @author James Forward | ||
| */ | ||
| @Requires(property = "global.datasources.data-source-properties") |
There was a problem hiding this comment.
I think better name would be global-data-source-properties
There was a problem hiding this comment.
@dstepanov The idea was to have global at a higher level and then have the usual datasources properties underneath, which makes it extensible and more inline with the current setup, since these data source properties are already set at datasources.<datasourcename>.data-source-properties. For example, if we did want to set other properties that would usually be under datasources.<datasourcename>, such as a global username, it would be global.datasources.username
| */ | ||
| @Requires(property = "global.datasources.data-source-properties") | ||
| @Singleton | ||
| public class GlobalDatasourceConfigModifier implements BeanCreatedEventListener<DatasourceConfiguration> { |
There was a problem hiding this comment.
Pls make final, package private and annotate @Internal
| */ | ||
| @Requires(property = "global.datasources.data-source-properties") | ||
| @ConfigurationProperties("global.datasources") | ||
| public class GlobalDatasourceProperties { |
There was a problem hiding this comment.
Pls make final and add javadoc to methods
| @Requires(property = "global.datasources.data-source-properties") | ||
| @ConfigurationProperties("global.datasources") | ||
| public class GlobalDatasourceProperties { | ||
| private Map<String, String> dataSourceProperties = new HashMap<>(); |
There was a problem hiding this comment.
Why this has to be mutatable?
| } | ||
| } | ||
|
|
||
| Map<String, ?> getIndividualDsProperties() { |
There was a problem hiding this comment.
@dstepanov The dbcp implementation needs to access the properties to work properly
|
Instead of the listener you should inject the global conf and apply the settings in the post construct method. |
3 participants
Right now this PR allows globally setting values in
data-source-properties, viaglobal.datasources.data-source-properties. We find this to be extremely helpful when enforcing organisational standards - for example, providing helm charts which provide these values out of the box which force connection pinning on PostgreSQL viaassumeMinServerVersionand forcing theApplicationNameto match the provisioned microservice.Naturally we could improve this in future to also allow global configuration of other properties rather than just the
data-source-propertiesmap, but this is a nice start