-
Notifications
You must be signed in to change notification settings - Fork 968
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
Add a config.strategy to enable overwrite of properties from env vars #620
Changes from all commits
d9d1f0a
5c04377
30b6556
7fe4788
e302b9a
c14bb77
4571c79
87dc17d
3a9a1d8
90e2465
4937ee4
fd0e418
4fad113
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 | ||
---|---|---|---|---|
|
@@ -32,6 +32,7 @@ | |||
* For use only by the {@link com.typesafe.config} package. | ||||
*/ | ||||
public class ConfigImpl { | ||||
private static final String ENV_VAR_OVERRIDE_PREFIX = "CONFIG_FORCE_"; | ||||
|
||||
private static class LoaderCache { | ||||
private Config currentSystemProperties; | ||||
|
@@ -360,6 +361,43 @@ public static void reloadEnvVariablesConfig() { | |||
EnvVariablesHolder.envVariables = loadEnvVariables(); | ||||
} | ||||
|
||||
|
||||
|
||||
private static AbstractConfigObject loadEnvVariablesOverrides() { | ||||
Map<String, String> env = new HashMap(System.getenv()); | ||||
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 don't mean to bike shed -- especially as I've seen you've written tests against this. I just though I'd add a suggestion which could (1) perhaps make the different override cases more easily scannable/readable and (2) make the test cases easier: Instead of conflating the 'System.getenv()' call within the same method which does the override logic, you might consider having something like: static String envVariableAsProperty(envVariable : String, envVarOverridePrefix : String) : String = { which you can then just test via: assertEquals(envVariableAsProperty("PREFIX_A_B__C", "PREFIX_"), "a.b-c") etc 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. it makes totally sense, thanks for pushing me out from my laziness :-) |
||||
Map<String, String> result = new HashMap(System.getenv()); | ||||
|
||||
for (String key : env.keySet()) { | ||||
if (key.startsWith(ENV_VAR_OVERRIDE_PREFIX)) { | ||||
result.put(ConfigImplUtil.envVariableAsProperty(key, ENV_VAR_OVERRIDE_PREFIX), env.get(key)); | ||||
} | ||||
} | ||||
|
||||
return PropertiesParser.fromStringMap(newSimpleOrigin("env variables overrides"), result); | ||||
} | ||||
|
||||
private static class EnvVariablesOverridesHolder { | ||||
static volatile AbstractConfigObject envVariables = loadEnvVariablesOverrides(); | ||||
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 guess we need to make ConfigFactory.invalidateCaches clear this
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. done |
||||
} | ||||
|
||||
static AbstractConfigObject envVariablesOverridesAsConfigObject() { | ||||
try { | ||||
return EnvVariablesOverridesHolder.envVariables; | ||||
} catch (ExceptionInInitializerError e) { | ||||
throw ConfigImplUtil.extractInitializerError(e); | ||||
} | ||||
} | ||||
|
||||
public static Config envVariablesOverridesAsConfig() { | ||||
return envVariablesOverridesAsConfigObject().toConfig(); | ||||
} | ||||
|
||||
public static void reloadEnvVariablesOverridesConfig() { | ||||
// ConfigFactory.invalidateCaches() relies on this having the side | ||||
// effect that it drops all caches | ||||
EnvVariablesOverridesHolder.envVariables = loadEnvVariablesOverrides(); | ||||
} | ||||
|
||||
public static Config defaultReference(final ClassLoader loader) { | ||||
return computeCachedConfig(loader, "defaultReference", new Callable<Config>() { | ||||
@Override | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
sbt.version=1.2.7 | ||
sbt.version=1.2.8 |
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.
if the value of the property is not a boolean this will throw an ugly and likely-mysterious exception right? might want to catch it and say something like "the value of OVERRIDE_WITH_ENV_PROPERTY_NAME must be a boolean not '%s'"
Not sure if we already have this bug when looking at any other env var values
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.
I've deliberately choose to use Boolean.parseBoolean because it doesn't throw exceptions.
Basically it always return false unless the string is (case-insensitive)
true
.