making possible to override environment variables by system properties #875

nykolaslima · 2014-11-11T17:14:18Z

When we are deploying in an environment like Amazon, we may need to update an environment variable on demand. Nowadays, we need to change the environment.properties in order to make this happen.

This pull request, allow us to override this properties using system properties.

We only allow user's to override properties that exists in environment.properties and do not allow to define only in system property.

#environment.properties
my.prop hello

//System.setProperty("my.prop", "bye");
environment.get("my.prop"); //bye
//System.setProperty("not.in.properties", "foo")
environment.get("not.in.properties"); //throw NoSuchElementException

nykolaslima · 2014-11-11T17:23:19Z

vraptor-core/pom.xml

@@ -63,6 +63,12 @@
 			<artifactId>xstream</artifactId>
 			<version>1.4.7</version>
 		</dependency>
+


Should I create a ValidationUtil class to do this?

I believe that notBlank method is something that we can use in many parts of the project.

We can use guava's Strings class:

http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Strings.html#isNullOrEmpty(java.lang.String)

On Tue Nov 11 2014 at 3:23:23 PM Nykolas Laurentino de Lima <
notifications@github.com> wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

Should I create a ValidationUtil class to do this?

I believe that notBlank method is something that we can use in many parts
of the project.

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20164752.

By the way, I think this feature is great. And I also believe that during
development environment, the properties should be reloaded for every
request.

On Tue Nov 11 2014 at 3:26:25 PM Chico Sokol chico.sokol@gmail.com wrote:

We can use guava's Strings class:

http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Strings.html#isNullOrEmpty(java.lang.String)

On Tue Nov 11 2014 at 3:23:23 PM Nykolas Laurentino de Lima <
notifications@github.com> wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

Should I create a ValidationUtil class to do this?

I believe that notBlank method is something that we can use in many
parts of the project.

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20164752.

Yes, use @csokol suggestion. We can't add a new jar with a lot of classes only because one method.

Thanks @csokol. I'll do this!

On Tuesday, November 11, 2014, csokol notifications@github.com wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

We can use guava's Strings class:
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Strings.html#isNullOrEmpty(java.lang.String)
… <#1499fe486c772a65_>
On Tue Nov 11 2014 at 3:23:23 PM Nykolas Laurentino de Lima <
notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote: In
vraptor-core/pom.xml: > @@ -63,6 +63,12 @@ >
xstream > 1.4.7 >

Should I create a ValidationUtil class to do this? I believe that
notBlank method is something that we can use in many parts of the project.
— Reply to this email directly or view it on GitHub <
https://github.com/caelum/vraptor4/pull/875/files#r20164752>.

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20164940.

Take it easy @garcia-jj. Already understood.

On Tuesday, November 11, 2014, Otávio Garcia notifications@github.com
wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

Yes, use @csokol https://github.com/csokol suggestion. We can't add a
new jar with a lot of classes only because one method.

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20165131.

@csokol About reload, sounds too fair. And its not too hard to implement. May we can do this feature in next pull request. What U think?

Yes, I think it should be in different PR.

On Tue Nov 11 2014 at 3:31:57 PM Otávio Garcia notifications@github.com
wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

@csokol https://github.com/csokol About reload, sounds too fair. And
its not too hard to implement. May we can do this feature in next pull
request. What U think?

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20165314.

garcia-jj · 2014-11-11T17:38:26Z

Just labeling this pull request conflicts with #821, so #821 needs to be refactored before merge into master.

@nykolaslima What you think to add some docs about this search priority?

nykolaslima · 2014-11-11T17:42:34Z

@garcia-jj sure! Doing this right now..

nykolaslima · 2014-11-11T17:50:37Z

Done. Can you review please @garcia-jj? My english isn't so good.

garcia-jj · 2014-11-11T17:54:58Z

After changes I think are ready to merge. Lets @Turini and @lucascs review the code.

I think that can be merged in this current release because there is no public API change, @Turini .

nykolaslima · 2014-11-11T17:57:41Z

Fixed. Thanks! Let's wait for review 👍

Turini · 2014-11-12T11:39:09Z

vraptor-site/content/en/docs/environment.html

+If you have a common properties for other environments you can create a file called `environment.properties` with these common properties. So VRaptor will ask for properties first in the environment file, and if not found, will ask in the `environment.properties` file.
+
+You can also override properties using System Properties. Properties defined by System Properties will override any other definitions.
+Note: You can only use System Properties to override existing properties in property files and not to define a new one.


@nykolaslima, can you just add an simple example?
I think It'll help users to get the idea. What do you think?

I think it would be nice...Gonna do it!

Turini · 2014-11-12T11:42:02Z

I think that can be merged in this current release because there
is no public API change, @Turini .

👍

nykolaslima · 2014-11-12T14:47:41Z

@Turini what do you think now?

nykolaslima · 2014-11-12T15:29:11Z

@Turini done. If you have any other suggestion please let me know. 👍

…em-property making possible to override environment variables by system properties

Turini · 2014-11-12T15:52:32Z

thank you @nykolaslima

nykolaslima · 2014-11-12T16:07:39Z

No problem. Thank you!

making possible to override environment variables by system properties

a715aae

nykolaslima reviewed Nov 11, 2014
View reviewed changes

Turini added the enhancement label Nov 11, 2014

removing commons-lang and using guava

ef66086

adding docs about overriding properties with system properties

deaebc8

typo

2a26b12

Turini reviewed Nov 12, 2014
View reviewed changes

adding example about override environment variables

f037338

doc improvements suggested by @Turini

4eda003

Turini added a commit that referenced this pull request Nov 12, 2014

Merge pull request #875 from nykolaslima/override-environment-by-syst…

a661389

…em-property making possible to override environment variables by system properties

Turini merged commit a661389 into caelum:master Nov 12, 2014

nykolaslima deleted the override-environment-by-system-property branch November 12, 2014 16:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

making possible to override environment variables by system properties #875

making possible to override environment variables by system properties #875

nykolaslima commented Nov 11, 2014

nykolaslima Nov 11, 2014

csokol Nov 11, 2014

csokol Nov 11, 2014

garcia-jj Nov 11, 2014

nykolaslima Nov 11, 2014

nykolaslima Nov 11, 2014

garcia-jj Nov 11, 2014

csokol Nov 11, 2014

garcia-jj commented Nov 11, 2014

nykolaslima commented Nov 11, 2014

nykolaslima commented Nov 11, 2014

garcia-jj commented Nov 11, 2014

nykolaslima commented Nov 11, 2014

Turini Nov 12, 2014

nykolaslima Nov 12, 2014

Turini commented Nov 12, 2014

nykolaslima commented Nov 12, 2014

nykolaslima commented Nov 12, 2014

Turini commented Nov 12, 2014

nykolaslima commented Nov 12, 2014

making possible to override environment variables by system properties #875

making possible to override environment variables by system properties #875

Conversation

nykolaslima commented Nov 11, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

@@ -63,6 +63,12 @@ xstream 1.4.7

Choose a reason for hiding this comment

@@ -63,6 +63,12 @@ xstream 1.4.7

Choose a reason for hiding this comment

Choose a reason for hiding this comment

@@ -63,6 +63,12 @@ xstream 1.4.7

Choose a reason for hiding this comment

@@ -63,6 +63,12 @@ xstream 1.4.7

Choose a reason for hiding this comment

Choose a reason for hiding this comment

@@ -63,6 +63,12 @@ xstream 1.4.7

garcia-jj commented Nov 11, 2014

nykolaslima commented Nov 11, 2014

nykolaslima commented Nov 11, 2014

garcia-jj commented Nov 11, 2014

nykolaslima commented Nov 11, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Turini commented Nov 12, 2014

nykolaslima commented Nov 12, 2014

nykolaslima commented Nov 12, 2014

Turini commented Nov 12, 2014

nykolaslima commented Nov 12, 2014

@@ -63,6 +63,12 @@
xstream
1.4.7

@@ -63,6 +63,12 @@
xstream
1.4.7

@@ -63,6 +63,12 @@
xstream
1.4.7

@@ -63,6 +63,12 @@
xstream
1.4.7

@@ -63,6 +63,12 @@
xstream
1.4.7