Skip to content
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

Multi-Profile Support #81

Merged
merged 5 commits into from
Jul 10, 2023
Merged

Multi-Profile Support #81

merged 5 commits into from
Jul 10, 2023

Conversation

SentryMan
Copy link
Collaborator

Now can set the profiles and load multiple files based on that.

it can be done in three ways

  1. Setting a CONFIG_PROFILES env variable
  2. Adding a -Dconfig.profiles to the command-line
  3. Having a config.profiles property in a config file

Fixes #79

final var profile = loadContext.eval(path);
loadWithExtensionCheck("application-" + profile + ".properties");
loadWithExtensionCheck("application-" + profile + ".yaml");
loadWithExtensionCheck("application-" + profile + ".yml");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this in but I think I'm noticing something subtle about loadWithExtensionCheck() ... which is that it will not load the FILE if the RESOURCE is found.

So I'm pondering:

  • For this case we might want to try and load RESOURCE and FILE (regardless if the RESOURCE is found)
  • Ordering wise we might want to load all the RESOURCE for all the profiles first, and then load all the FILE for all the profiles after (ordering here in terms of file properties overriding resource properties).

So I'm pondering those 2 subtle things. I'll merge this into master now though, get a new coffee and ponder those 2 things ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typically, my property files are only in src/main/resources so I'm not bothered

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that was a feature tbh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I get that. 99% of the time there will not be FILE & RESOURCE defined (but for that 1% case I think we need to be consistent. Config generally loads all the FILEs afterwards as those are the "overrides" to the RESOURCE "defaults".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that was a feature tbh

Yeah, might be a bug in my eyes :)

That is, when we have a properties/yaml as FILE (so externally supplied) in theory it's put there to override RESOURCE (application supplied defaults) properties.

So Config as I see it really should be loading any FILE, and loading it after RESOURCE properties have been loaded. So it might be a feature but right now I feel it might be more a subtle "historic bug". It might be ok because the use cases typically do not have both RESOURCE and FILE.

It is tempting to change that to try to load FILE regardless if there was a RESOURCE found and loaded. Pondering, Hmm ...

@rbygrave rbygrave merged commit fc7d09c into avaje:master Jul 10, 2023
2 checks passed
@rbygrave rbygrave added this to the 3.5 milestone Jul 10, 2023
@SentryMan SentryMan deleted the profiles branch July 10, 2023 23:48
rbygrave added a commit that referenced this pull request Jul 11, 2023
Config generally loads RESOURCE source first, and FILE source later. This allows the FILE source properties to override the RESOURCE source properties in a predictable manner.
rbygrave added a commit that referenced this pull request Jul 11, 2023
rbygrave added a commit that referenced this pull request Jul 11, 2023
This change ensures that for any additional profiles defined in loadMain() the RESOURCE is loaded.

- Keeps a track of the profile resources loaded
- Ensures for any additional profiles defined via loadMain() that those RESOURCE are also loaded

Background:
Config generally loads RESOURCE source first, and FILE source later. This allows the FILE source properties to override the RESOURCE source properties in a predictable manner.
rbygrave added a commit that referenced this pull request Jul 11, 2023
…rces

#81 Adjust load ordering of profiles (take2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multi-Profile support
3 participants