Use Maven/Gradle profile(s) as default Spring profile(s) #3224

erikkemperman · 2016-03-19T21:56:40Z

Sorry for the many PRs, I will stop now :-)

As discussed, among other places, in issue #3180, this promotes the profiles used with Maven/Gradle to be the default profile(s) in Spring -- unless an explicit spring.profiles.active is given, of course.

This means that mvn -Pprod package && java -jar target/foo.war does what you'd expect without the need for arguments. Likewise when deploying the war.

It might be too late for the 3.0 release...? Although I kind of feel that it belongs together with the change that made the dev and prod assets mutually exclusive in the war file.

I did not change the default profile to be different for run or package goals, as was suggested: I am not sure this is even possible with Maven.

I briefly looked into changing the war filename depending on the profiles used, as was suggested in the linked issue, but the generators for AWS, Cloudfoundry, and Heroku seem to only expect a single war file to be present: Cloudfoundry and Heroku use *.war, while AWS (arbitrarily?) picks the last *.war.original file returned by fs.readdirSync.

By the way, shouldn't the uploaded war for those generators always be the .original one, also for Heroku and Cloudfoundry?

deepu105 · 2016-03-20T03:14:08Z

I know for sure that .original us required for AWS but not sure of heroku
and CF. I can check CF but someone needs to check heroku
On 20 Mar 2016 05:56, "Erik Kemperman" notifications@github.com wrote:

Sorry for the many PRs, I will stop now :-)

As discussed, among other places, in issue #3180
#3180, this
promotes the profiles used with Maven/Gradle to be the default profile(s)
in Spring -- unless an explicit spring.profiles.active is given, of
course.

This means that mvn -Pprod && java -jar target/foo.war does what you'd
expect without the need for arguments. Likewise when deploying the war.

It might be too late for the 3.0 release...? Although I kind of feel that
it belongs together with the change that made the dev and prod assets
mutually exclusive in the war file.

I did not change the default profile to be different for run or package
goals, as was suggested: I am not sure this is even possible with Maven.

I briefly looked into changing the war filename depending on the profiles
used, as was suggested in the linked issue, but the generators for AWS,
Cloudfoundry, and Heroku seem to only expect a single war file to be
present: Cloudfoundry and Heroku use *.war, while AWS (arbitrarily?)
picks the last *.war.original file returned by fs.readdirSync.

By the way, shouldn't the uploaded war for those generators not always be

the .original one, also for Heroku and Cloudfoundry?

You can view, comment on, or merge this pull request online at:

#3224
Commit Summary

Use Maven/Gradle profile(s) as default Spring profile(s)

File Changes

M generators/server/index.js
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-0
(2)

M generators/server/templates/_build.gradle
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-1
(2)

M generators/server/templates/_pom.xml
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-2
(18)

M generators/server/templates/gradle/_profile_dev.gradle
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-3
(13)

M generators/server/templates/gradle/_profile_prod.gradle
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-4
(12)

M
generators/server/templates/src/main/java/package/_Application.java
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-5
(9)

M
generators/server/templates/src/main/java/package/_ApplicationWebXml.java
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-6
(7)

A
generators/server/templates/src/main/java/package/config/_BuildProperties.java
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-7
(56)

A
generators/server/templates/src/main/resources/build-properties.xml
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-8
(6)

M test/test-creation.js
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-9
(2)

Patch Links:

https://github.com/jhipster/generator-jhipster/pull/3224.patch

https://github.com/jhipster/generator-jhipster/pull/3224.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#3224

deepu105 · 2016-03-20T03:15:46Z

And yes this should be part if 3.0
On 20 Mar 2016 11:14, d4udts@gmail.com wrote:

I know for sure that .original us required for AWS but not sure of heroku
and CF. I can check CF but someone needs to check heroku
On 20 Mar 2016 05:56, "Erik Kemperman" notifications@github.com wrote:

Sorry for the many PRs, I will stop now :-)

As discussed, among other places, in issue #3180
#3180, this
promotes the profiles used with Maven/Gradle to be the default profile(s)
in Spring -- unless an explicit spring.profiles.active is given, of
course.

This means that mvn -Pprod && java -jar target/foo.war does what you'd
expect without the need for arguments. Likewise when deploying the war.

It might be too late for the 3.0 release...? Although I kind of feel that
it belongs together with the change that made the dev and prod assets
mutually exclusive in the war file.

I did not change the default profile to be different for run or package
goals, as was suggested: I am not sure this is even possible with Maven.

I briefly looked into changing the war filename depending on the profiles
used, as was suggested in the linked issue, but the generators for AWS,
Cloudfoundry, and Heroku seem to only expect a single war file to be
present: Cloudfoundry and Heroku use *.war, while AWS (arbitrarily?)
picks the last *.war.original file returned by fs.readdirSync.

By the way, shouldn't the uploaded war for those generators not always be

the .original one, also for Heroku and Cloudfoundry?

You can view, comment on, or merge this pull request online at:

#3224
Commit Summary

Use Maven/Gradle profile(s) as default Spring profile(s)

File Changes

M generators/server/index.js
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-0
(2)

M generators/server/templates/_build.gradle
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-1
(2)

M generators/server/templates/_pom.xml
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-2
(18)

M generators/server/templates/gradle/_profile_dev.gradle
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-3
(13)

M generators/server/templates/gradle/_profile_prod.gradle
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-4
(12)

M
generators/server/templates/src/main/java/package/_Application.java
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-5
(9)

M
generators/server/templates/src/main/java/package/_ApplicationWebXml.java
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-6
(7)

A
generators/server/templates/src/main/java/package/config/_BuildProperties.java
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-7
(56)

A
generators/server/templates/src/main/resources/build-properties.xml
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-8
(6)

M test/test-creation.js
https://github.com/jhipster/generator-jhipster/pull/3224/files#diff-9
(2)

Patch Links:

https://github.com/jhipster/generator-jhipster/pull/3224.patch

https://github.com/jhipster/generator-jhipster/pull/3224.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#3224

deepu105 · 2016-03-20T04:37:39Z

generators/server/templates/gradle/_profile_dev.gradle

 processResources {
+    filesMatching('**/.build-properties.xml') {


I hate to have an xml for this its kind of ugly :( cant we do the same to the profiles .yml file? Or using the application.yml file?

deepu105 · 2016-03-20T04:40:36Z

I had a look and im not sure. I dont like having additional xml and a class for this, isnt there a simpler way?

Im not sure if this additional complexity which is a bit non standard for normal spring apps IMO is worth the value provided which is letting the user run without passing a profile.

Any way we need to see if there is any simpler alternative to this

erikkemperman · 2016-03-20T07:43:49Z

Absolutely agree that this is not very pretty! This is why I made it a hidden file -- slightly embarrassing :-)

I have tried to find a nicer way to do this, but couldn't find one. If anyone has ideas... Please let me know!

For what it's worth though, putting this in the application.yml definitely won't work; it has to be done before spring parses those yaml files, because it will be used to determine which of those to load.

erikkemperman · 2016-03-20T07:52:38Z

Of course the same mechanism, with almost no extra effort, could now be used to deliver more properties from build to runtime. For my own application I'm thinking to include build tool (maven/gradle), build date, git revision (hash of latest commit), ...

deepu105 · 2016-03-20T08:34:04Z

May be lets take some time and see what can be done, this is not a blocker
for 3.0 anyway so we can take our time

Thanks & Regards,
Deepu

On Sun, Mar 20, 2016 at 3:52 PM, Erik Kemperman notifications@github.com
wrote:

Of course the same mechanism, with almost no extra effort, could now be
used to deliver more properties from build to runtime. For my own
application I'm thinking to include build tool (maven/gradle) date, git
revision (hash of latest commit), ...

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#3224 (comment)

deepu105 · 2016-03-20T09:02:18Z

lets do this after 3.0 so im gonna put this on hold

erikkemperman · 2016-03-20T09:17:36Z

Well, despite maybe being subject to a better implementation, I think that it functionally accomplishes a useful detail -- and it shouldn't get in the way of anything else, I don't think, contact surface with rest of the code is unobtrusive and limited to the main and webxml classes.

But fair enough, please let me know if / when to pick this up, or if anyone sees better ways to do this!

erikkemperman · 2016-03-20T09:38:58Z

Maybe I should mention one other implementation I considered, but decided against: the @variable@ subs could of course be done on (a) Java file(s) directly, without an extra xml file or properties class. But we were interpolating/filtering the xml files anyway (for @loglevel@) and having the class seems preferable as it's a natural place to handle fallback for unsubstited values (e.g running in IDE).

deepu105 · 2016-03-20T09:51:56Z

lets wait and see if someone has a better idea

Thanks & Regards,
Deepu

On Sun, Mar 20, 2016 at 5:39 PM, Erik Kemperman notifications@github.com
wrote:

Maybe I should mention one other implementation I considered, but decided
against: the @variable@ subs could of course be done on (a) Java file(s)
directly, without an extra xml file or properties class. But we were
interpolating/filtering the xml files anyway (for @loglevel@) and having
the class seems preferable as it's a natural place to handle fallback for
unsubstited values (e.g running in IDE).

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#3224 (comment)

erikkemperman · 2016-03-24T22:25:00Z

Minor FYI: removed a now unneeded import from ApplicationWeb.xml, and rebased onto latest master (had to adapt to a commit to make gradle/protractor test pass).

I would still love to hear if anyone has ideas on how to improve on this implementation!

erikkemperman · 2016-03-25T00:48:50Z

Travis is flaky; restarted the failing test and now it passed.

deepu105 · 2016-03-30T06:21:00Z

@jhipster/developers guys we need to progress on this, any comments from you guys?

atomfrede · 2016-03-31T15:52:01Z

Looks good to me. I have no feeling against xml, in case it stays as small as possible

PierreBesson · 2016-03-31T16:03:09Z

I'm in favor of including this. It will be great for on-boarding because users would expect the app to start in prod when packaging a jar for prod. Also if spring.profiles.active always take precedence it should not cause issues.
It will also be very helpful as we started to make more use of profiles recently with no-swagger and no-liquibase.

erikkemperman · 2016-03-31T18:15:30Z

Rebased again and fixed an inaccurate comment in BuildProperties.java.

deepu105 · 2016-04-01T05:35:44Z

Im not a spring boot expert but I was wondering. Spring boot does support loading stuff from an external application.properties or application.yml file as stated here https://docs.spring.io/spring-boot/docs/current/reference/html/boot-features-external-config.html

may be we could figure something out using that and the springs various annotations to load specific stuff based on things available in classpath or environment. Thats how spring does most of the autoconfiguration magic isnt it?

erikkemperman · 2016-04-01T07:15:15Z

Well, I'm no spring expert either, so maybe I'm missing something...

But my thinking was we can't use spring's autoconfig voodoo for this particular case, because we need it done before spring looks for external .properties or .yaml files. This is because the profile we are loading here will be used to determine which external config files spring must load, e.g. application-dev.yml or application-prod.yml.

I could change it from .build-properties.xml to a regular properties file if you prefer, I chose xml just because the maven pom was already set up to do filtering (which is its odd name for what is actually substitution) on xml files.

But I think we'd still need to load it manually, and provide a fallback for unsubstuted values, which makes me think having a separate class (BuildProperties.java) is actually a pretty natural fit for the purpose.

deepu105 · 2016-05-12T05:22:05Z

@jdubois @erikkemperman Ok so I did some more digging to see what was causing the original issue in our setup where the below

spring:
    profiles:
        active: prod

in application.yml was not working. and I found the cause, seems like we were setting addDefaultProfile(app, source); before the environment was loaded and hence dev profile was always taking precedence over application.yml see this order

So I guess the solution here is very simple and much cleaner. we can set the below 2 properties in our application.yml

spring:
    profiles:
        active: #spring.profile.active#
        default: dev

and we need to replace #spring.profile.active# with prod or dev based on what profile is used when running maven/gradle. and spring will automatically load the appropriate application-{profile}.yml

So this means we wouldnt need an xml and a BuildProperties class and IMO its much cleaner approach and kind of the spring way.

I tried this on a generated project and seems to work.

I foresee once issue with replacing content using maven/gradle with this PR as well as with my approach above. When we run the build with a specific profile it will replace the placeholder in the file, if we run the build with another profile later there is no more placeholder to replace. @erikkemperman how did you handle that, I dont see anything specific for that in the PR?

erikkemperman · 2016-05-12T05:32:19Z

@deepu105 That sounds good! So you figured out how to call addDefaultProfile before the env is loaded?

Note that maven/gradle don't change the file itself, but substitute while copying to target/build. So that shouldn't be an issue.

Without the properties class, though, how to handle fallback when running without maven (in IDE)?

erikkemperman · 2016-05-12T05:35:15Z

Also, will it still be possible to override the profile after the war is generated (see @mraible's comment above)?

deepu105 · 2016-05-12T07:00:19Z

@erikkemperman we dont have to call addDefaultProfile as spring.profile.default should take care of that.

If maven/gradle doesnt change the source then it should be good, then we have to pass the profile as the -Dspring.profiles.active similar to today to make correct profile load during bootRun and while running from IDE, since its handled already today we can retain the same and do the replacing of the @active# property only during mvn package or gradle bootRepackage

I think we cant use the dev war for prod and vice versa due to the PR you did to move stuff to the target folder. Anyway we discussed that and was decided that way. So no point going back on it again. I agree it was a nice feature but it was also adding a lot of bloat to the built package

erikkemperman · 2016-05-12T07:12:14Z

@deepu105 Ah, sorry, I had missed you put profiles.default in the yaml as well.

Maven/gradle doesn't change the source, at least in my setup (I hiked along with how it replaces @logback.loglevel@, which would also have been broken otherwise).

You're right about the front-end implications of dev vs prod of course, but it was my understanding that there are now additional profiles that affect backend only? This might also be true for user-defined profiles (I know I have those in my project, at least). Also, there seems to be an end-point for getting the active profiles now, so except for how the front-end code is packaged (i.e., concatenated, minified, revisioned or not) there could still be front-end switches on profile.

But as long as the override with -Dspring.profiles.active or --spring.profiles.active still works this should all work out fine I guess.

About fallback when running in IDE, yes we should then set --spring.profiles.active manually if it is not dev. That would work. But I was more worried about what spring will do when it tries to load a profile called #spring.profile.active# which is what it would I expect it to do when it parses the source application.yml rather than the substituted one in build or target folder?

All in all I absolutely agree that this sounds like a much cleaner way to achieve the desired effect. Kudos! If / when you have a branch that you'd be willing to share, I would love to try it out!

deepu105 · 2016-05-12T07:23:49Z

# is comment in yaml so spring takes it as empty :)

BTW seems like the profiles.default is not officially supported by spring boot and im tryig to see if there is another way to set a default profile when application.property is not set

erikkemperman · 2016-05-12T07:25:11Z

No need to shout :-) (But I see what happened there, with the hash tag making it H1)

deepu105 · 2016-05-12T07:45:37Z

LOL.

Ok found a way to set the default profile elegantly :)

@erikkemperman I can do a PR with the settings to change the way default profiles are done. But im a gradle guy and my maven skills suck, so could you help to do the replacing #active# part for maven?

erikkemperman · 2016-05-12T07:59:49Z

@deepu105 Well, I'm not sure how we could override the default delimiters @ for values to be substituted... The docs mention an alternative ${..} notation but that wouldn't be a comment in yaml.

erikkemperman · 2016-05-12T08:19:43Z

@deepu105 Okay, I think it will require moving how it is done currently into a separate plugin section, but it looks like we can then specify our own delimiter.

I will try this in a separate project, and will let you know what I find.

jdubois · 2016-05-12T08:46:31Z

This looks much, much better than the XML file :-)
@deepu105 @erikkemperman do you need any help? Looks like you're solving this perfectly!

erikkemperman · 2016-05-12T09:01:59Z

@deepu105 I think something like this would do the trick. It assumes that we also change the delimiter for the logback level, so beware that that would have to change in gradle as well. Alternative, we could use both @ and # as delimeters, I suppose/

Either way, I trust this gives you all you need to complete the PR? If not, let me know.

@jdubois Agreed, if Deepu's approach works it is much better. Thank you guys so much for not giving up on this!!

deepu105 · 2016-05-12T09:03:27Z

@erikkemperman I will try today else I can do only next week as im going on a short vacation tomorrow

jdubois · 2016-05-12T09:04:02Z

@erikkemperman yes and I'm very happy :-)
@deepu105 tell me if you need help! We need this for our next version :-)
And don't forget to update the doc accordingly :-)

deepu105 · 2016-05-12T09:06:48Z

Ill commit what i did so far in a PR any way, so that you guys can try it out if required.
@jdubois The ApplicationWebXml.java file has a method setting a default profile

@Override
    protected SpringApplicationBuilder configure(SpringApplicationBuilder application) {
        return application.profiles(addDefaultProfile())
            .sources(Application.class);
    }

But this doesnt seem to be triggered during bootRun or when run from war, when is this trigerred? is this for deploying to tomcat/jboss etc?

jdubois · 2016-05-12T09:08:37Z

@deepu105 to be honnest, I'm not sure if it's being used :-)
I coded this at the very beginning of the project, at that time it was to replace the web.xml file, but as of now it looks like it's not doing anything.... You could try removing it? And that would also clean up the root package, that would be perfect.

deepu105 · 2016-05-12T09:12:06Z

Its working without this file, when I run from IDE, bootrun from console or when run as java -jar need to check deployment in tomcat

jdubois · 2016-05-12T09:13:10Z

@deepu105 then kill it, KILL IT 💯

erikkemperman · 2016-05-12T09:13:28Z

@deepu105 @jdubois I wondered the same thing, when I was working on this PR, but if I remember correctly this is indeed (only) called by servlet containers as an alternative to web.xml. The logic is completely analogous to the main application class, though, so not really hard to keep both equivalent.

jdubois · 2016-05-12T09:15:51Z

@erikkemperman that's also what I remember... stupid app servers! Then we might need to keep it, and validate that it works well with this new setup

erikkemperman · 2016-05-12T09:15:57Z

The documentation for the superclass SpringBootServletInitializer says

Note that a WebApplicationInitializer is only needed if you are building a war file and deploying it. If you prefer to run an embedded container then you won't need this at all.

deepu105 · 2016-05-12T09:19:50Z

Ok ill try to check it

deepu105 · 2016-05-12T10:35:02Z

First PR is done. we need to do the property replace from maven and gradle. for gradle its very easy as I said earlier. But I wont have time for few days for that

jdubois · 2016-05-12T12:10:41Z

@erikkemperman @deepu105 as work as begun on the property replace, are you OK to close this PR?

erikkemperman · 2016-05-12T12:21:15Z

Yup, I guess that's the way to go. Closing.

erikkemperman force-pushed the feature-build-profile branch from 6ffabb3 to a9a5336 Compare March 19, 2016 22:17

deepu105 reviewed Mar 20, 2016
View reviewed changes

deepu105 added status: needs-discussion JH 3.0 labels Mar 20, 2016

deepu105 removed the JH 3.0 label Mar 20, 2016

deepu105 added the pr: on-hold label Mar 20, 2016

deepu105 removed the pr: on-hold label Mar 24, 2016

erikkemperman force-pushed the feature-build-profile branch 2 times, most recently from b959cef to 67f0719 Compare March 24, 2016 22:11

erikkemperman force-pushed the feature-build-profile branch from 67f0719 to 42b3eaf Compare March 24, 2016 23:04

erikkemperman force-pushed the feature-build-profile branch from 42b3eaf to 166e8aa Compare March 31, 2016 10:59

erikkemperman force-pushed the feature-build-profile branch from 166e8aa to baa2365 Compare March 31, 2016 17:57

deepu105 mentioned this pull request May 12, 2016

Set default profile #3585

Merged

erikkemperman closed this May 12, 2016

jdubois modified the milestone: 3.3.0 May 13, 2016

erikkemperman deleted the feature-build-profile branch May 19, 2016 13:12

		processResources {
		filesMatching('**/.build-properties.xml') {

Use Maven/Gradle profile(s) as default Spring profile(s) #3224

Use Maven/Gradle profile(s) as default Spring profile(s) #3224

Conversation

erikkemperman commented Mar 19, 2016

deepu105 commented Mar 20, 2016

the .original one, also for Heroku and Cloudfoundry?

deepu105 commented Mar 20, 2016

the .original one, also for Heroku and Cloudfoundry?

deepu105 Mar 20, 2016

Choose a reason for hiding this comment

deepu105 commented Mar 20, 2016

erikkemperman commented Mar 20, 2016

erikkemperman commented Mar 20, 2016

deepu105 commented Mar 20, 2016

deepu105 commented Mar 20, 2016

erikkemperman commented Mar 20, 2016

erikkemperman commented Mar 20, 2016

deepu105 commented Mar 20, 2016

erikkemperman commented Mar 24, 2016

erikkemperman commented Mar 25, 2016

deepu105 commented Mar 30, 2016

atomfrede commented Mar 31, 2016

PierreBesson commented Mar 31, 2016

erikkemperman commented Mar 31, 2016

deepu105 commented Apr 1, 2016

erikkemperman commented Apr 1, 2016

deepu105 commented May 12, 2016

erikkemperman commented May 12, 2016

erikkemperman commented May 12, 2016

deepu105 commented May 12, 2016

erikkemperman commented May 12, 2016

deepu105 commented May 12, 2016 • edited

erikkemperman commented May 12, 2016

deepu105 commented May 12, 2016

erikkemperman commented May 12, 2016

erikkemperman commented May 12, 2016

jdubois commented May 12, 2016

erikkemperman commented May 12, 2016

deepu105 commented May 12, 2016

jdubois commented May 12, 2016

deepu105 commented May 12, 2016

jdubois commented May 12, 2016

deepu105 commented May 12, 2016

jdubois commented May 12, 2016

erikkemperman commented May 12, 2016

jdubois commented May 12, 2016

erikkemperman commented May 12, 2016 • edited

deepu105 commented May 12, 2016

deepu105 commented May 12, 2016

jdubois commented May 12, 2016

erikkemperman commented May 12, 2016

deepu105 commented May 12, 2016 •

edited

erikkemperman commented May 12, 2016 •

edited