Skip to content

Add prefix support for environment variables #3480

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

Closed
wants to merge 0 commits into from

Conversation

lucassaldanha
Copy link
Contributor

Creates an environmentPrefix property that is used to filter the
SystemEnvironmentPropertySource's map of environment
variables and create a new one only with the prefixed properties.
This new map is used on a new SystemEnvironmentPropertySource
that replaces the former one.

Fixes #3450

@lucassaldanha
Copy link
Contributor Author

@philwebb I didn't understand why the build failed. Looks like some error in Travis CI worker... Do you know what happened?

@snicoll
Copy link
Member

snicoll commented Jul 14, 2015

I don't think that's what we had in mind. We shouldn't change the existing PropertySource as anybody may want to look it up manually (either via the Environment or @Value).

We thought of applying that constraint for binding so that not the full OS environment is available as it may actually break it (see the discussion on the original issue).

My best guess is that that PropertySource replacement should occur when @ConfigurationProperties POJO are bound to the environment (there's a PostProcessor that does that). Do you want to give that a try and submit a new PR? (please push force on your branch rather than creating a new PR)

@lucassaldanha
Copy link
Contributor Author

@snicoll Ok! I'll adjust the code to match the behavior you described.

+ * for this application.
+ * @param environmentPrefix the prefix
+ */
public void setEnvironmentPrefix(String environmentPrefix) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@snicoll I'm not sure if this is the best way to configure the environment prefix. I'd like some opinions on that.

@lucassaldanha
Copy link
Contributor Author

@snicoll I believe that now it does what is is expected to do (considering the prefix only on binding).

I made a line note about the way that the environment prefix is being configured. Because I'm not sure if it's a good idea using the system properties to do that. Need some opinions.

@lucassaldanha lucassaldanha force-pushed the master branch 2 times, most recently from 5878dd5 to 479e829 Compare July 18, 2015 04:14
@lucassaldanha lucassaldanha changed the title Add prefix support for environment variables (Fixes #3450) Add prefix support for environment variables Jul 18, 2015
@lucassaldanha lucassaldanha force-pushed the master branch 4 times, most recently from 14f483b to 4e25623 Compare October 22, 2015 07:52
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 17, 2016
@snicoll
Copy link
Member

snicoll commented Oct 5, 2016

@lucassaldanha this PR was automatically closed because you made it from master and your changes went away when you push force on your fork. If you still want to contribute this change, please create a PR from a branch and not master. Thanks.

@snicoll snicoll added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add prefix support for environment variables
3 participants