In our project we created a starter which contains most general stuff, like database, feign clients and so on.
Now we want to configure this starter with different properties per stage.
Currently we created a file application-commons.yaml
in our starter and include this "profile" at spring.profiles.active, but this does not really scale well if we create more starters, or more applications. We would have to keep in mind to add all the profiles to all the services.
With this PR we are able to implement an AdditionalConfigLoader
, which returns the name of the starters "name", and then add the class to the spring.factories
: org.springframework.boot.context.config.AdditionalConfigLoader=example.AdditionalCommonConfigLoader
We could now create multiple files like commons.yaml
, commons-local.yaml
and so.
This PR relates to https://github.com/spring-projects/spring-framework/pull/26458 which adds the ability to load an additional file like application-commons.yaml
.
Comment From: wilkinsona
Thanks for the proposal. This is one possible way of implementing https://github.com/spring-projects/spring-boot/issues/24688, at least in part.
Comment From: raddatzk
I think this would resolve most issues of #24688.
- If two modules provide both
application.properties
nothing would change, but both could also provide their own files likemodule1.properties
andmodule2.properties
. Each module would then need to implement theAdditionalConfigLoader
- The files will be ordered using the
AnnotationAwareOrderComparator
that orders allAdditionalConfigLoader
first by an@PriorityOrdered
and second by@Ordered
with a precedence. All unannotated class will be put at the end in an arbitrary order. This might be a problem, I don't know if this order could change within multiple starts, it would be the developers task to make sure that there are no overlapping properties. Only way around I see is overloadingSpringFactoriesLoader.loadFactories
with an other comparator that orders all unannotated classes by name or something else. - Startup time should not be impaced affected, but I didn't do any measuments.
- It should be still backward compatible.
spring.config.import
should also not be impacted.
Comment From: raddatzk
Why is this pull request marked as draft?
Comment From: philwebb
I'm afraid I don't think that we can accept this pull-request in its current form. I feel like the we need to look at a different approach for #24688. I'm particularly concerned that having additional configuration only apply when spring.config.name
is not bound will be quite limiting.
If that's what you want for your current setup, it might be possible to do it with an EnvonmentPostProcessor
. If you have one that runs before the ConfigDataEnvironmentPostProcessor
then you'd be able to insert a PropertySource
with a spring.config.name
value populated via your AdditionalConfigLoader
interface. You can also add a spring.profiles.include
which will remove the need for the AdditionalProfileLoader
changes in Spring Framework.
I'll add a note to #24688 so we can consider this use-case when we work on the issue.
Thanks anyway for the pull-request.
Comment From: wilkinsona
@kevinraddatz I believe Brian marked it as a draft as you force-pushed changes to it several times after opening it. Each force-push notifies everyone watching the repository which adds a lot of noise. It also suggested that the changes were still a work in progress.
Comment From: raddatzk
Thank you for your reply, I understand your concerns. The idea to put this at such an early stage was to be able to also set spring.main
properties, ~~but I didn't think about risks, as a module also could allow allow-bean-definition-overriding
~~ but I didn't knew about the EnvironmentPostProcessor
. I will try this approach.
~~I will try your approach with the EnvironmentPostProcessor
, this seems to be enough for me.~~
Sorry for the noise I created, I didn't knew about this.