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

Feature Request: Add system:properties and system:env as sources #110

Closed
kevin-canadian opened this Issue Nov 14, 2014 · 6 comments

Comments

Projects
None yet
3 participants
@kevin-canadian
Contributor

kevin-canadian commented Nov 14, 2014

I would like to be able to specify system properties and/or environment variables as one potential source for properties in addition to files, especially when using the MERGE load type. Mainly I want to be able to conveniently override default values with system parameters.

Currently it is easy to add ALL system properties or environment variables to a config object. But I'd like to be able to specify the system as one potential source, rather than bloating my config object with everything.

Currently this can be done with a utility class, which checks if a property is in the config object before including new properties from the system. But it would be great if this was supported natively.

Specifically, I would like to be able to do something like the following.

@LoadPolicy(LoadType.MERGE)
@Sources({
  "system:properties",
  "system:env",
  "file:~/myconfig.properties",
  "file:/etc/myconfig.properties",
  "classpath:/myconfig.properties" })
public interface MyConfig extends Config {
  ...
}

In this case I would like the properties to be taken first from system properties, other from environment variables, otherwise from ~/myconfig.properties, etc.

@lviggiano

This comment has been minimized.

Owner

lviggiano commented Nov 14, 2014

👍 Good idea.

@lviggiano lviggiano closed this in 85436e7 May 6, 2016

lviggiano added a commit that referenced this issue May 6, 2016

Merge pull request #122 from gintau/110-sys-props-env-vars
#110 add system properties and enviroment variable as sources
@StFS

This comment has been minimized.

Contributor

StFS commented May 6, 2016

Hmm... that commit doesn't seem to do very much?

@lviggiano

This comment has been minimized.

Owner

lviggiano commented May 6, 2016

I merged this pull request #122:

a2b914d Revert LoadStrategyTest
083a539 Put FIRST and MERGE strategies into new test class
c6acd56 Add test system properties hot reload
5d5e855 make all tests pass
3f48e86 remove header template
85436e7 resolve #110

@StFS

This comment has been minimized.

Contributor

StFS commented May 6, 2016

ahh ok... the close activity just looked weird because it only referenced the last commit. Sorry about that.

@lviggiano

This comment has been minimized.

Owner

lviggiano commented May 6, 2016

No problem; myself would have been confused as well.

@lviggiano

This comment has been minimized.

Owner

lviggiano commented May 6, 2016

@StFS please double check the code and the feature to ensure your use case is covered. If you (or @gintau) have time, some documentation on owner-site module, would be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment