Add Support For Thin Jars in GAE #10420

SudharakaP · 2019-09-15T15:45:28Z

This adds support for unjar the "fat jar" within the staging folder, thus giving ability for users to fully leverage the GCP platform features.

Fixes #10362

Please make sure the below checklist is followed for Pull Requests.
Travis tests are green
Tests are added where necessary
Documentation is added/updated where necessary
Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

Only adds to maven build; work in progress

A previous pull request incorrectly removes the com.google.cloud.tools when gaeCloudSQLInstanceNeeded is set to false #10362

The new GAE with Java 11 builds on JAR; this has been done on the maven side.

A previous pull request incorrectly removes the com.google.cloud.tools when gaeCloudSQLInstanceNeeded is set to false

If this is not done the following error is created; yntaxError: Multi-line double-quoted string needs to be sufficiently indented (16:14) 14 | datasource: 15 | type: com.zaxxer.hikari.HikariDataSource > 16 | url: "jdbc:mysql://google/mysql?cloudSqlInstance=abstract-block-253023:us-central1:testjhipster | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This adds Gradle side support for thin jars

generators/gae/templates/pom-plugin.xml.ejs

SudharakaP · 2019-09-16T17:07:10Z

As per my discussion in GoogleCloudPlatform/app-maven-plugin#399 we need to change this approach. I will refactor the current pull request soon. 😄

SudharakaP · 2019-09-17T01:26:05Z

@saturnism : I've made the changes and added ludoch's workaround to remove the original jar. I think we are good go ahead with code review and merging.

saturnism · 2019-09-19T16:32:32Z

/cc @ludoch

quick que

w/ this patch, we still retain the JAR file, and in addition we have the unpacked directory
if i remove a resource/class from source, the unpacked directory will reflect that deletion as well?

SudharakaP · 2019-09-19T16:55:20Z

@saturnism :

As it stands currently, the JAR file is removed (not uploaded to GAE) as it's no longer useful since we unpacked it. Only the unpacked contents remain in staging directory.
At deployment to GAE, the unpacked directory will reflect the deletion; each time you deploy the staging directory is rebuilt to reflect the new contents.

Does these clarify your questions? 😄

saturnism · 2019-09-24T20:53:40Z

@SudharakaP thanks for confirming. Understood. It would be really nice if it was also able to retain the original JAR, but prob not easy to do do right now.

@ludoch @loosebazooka - perhaps more we can do on the plugin side to make this type of work flow much easier?

generators/gae/templates/app.yaml.ejs

SudharakaP · 2019-09-25T01:00:25Z

@SudharakaP thanks for confirming. Understood. It would be really nice if it was also able to retain the original JAR, but prob not easy to do do right now.

I think there's a slight misunderstanding here. So initially we have our original jar (we'll call it the fatjar). This fat jar is copied to the staging directory and then deployed by the appengine plugins. What we did is to unjar this fat jar within the staging directory (the original fat jar outside the staging directory is still there) and then tell appengine plugin to deploy. But appengine plugin when told to deploy not only deploy it but does the staging again. This copies the fatjar back into the appengine-staging directory. This already extracted fatjar within the staging directory is the only one that is deleted (using the workaround as suggested by ludoch). So to answer your original question the actual fatjar generated by spring boot remains within the target directory. Does this make sense? 😄

On another note what probably should be done to enhance the appengine maven and gradle plugins is to have an option to only deploy without doing the staging; or any means to get rid of the workaround to remove the fatjar in the staging directory. Furthermore the documentation use the term thin jar which probably gives a wrong idea where we can just upload a jar without any dependencies which I initially tried to do using spring-boot-thin-launcher which is the wrong approach.

SudharakaP · 2019-10-04T06:31:46Z

With reference to #10533, I've just added the -Xmx value for appengine entry-point.

saturnism · 2019-10-10T20:59:51Z

we shouldn't hard code Xmx here, since instances are different and heap size depends on the instance size.

/cc @ludoch @loosebazooka this is getting really hard to do a simple deployment in an optimized way. tbh, the plugin itself should do the JAR unpacking, and/or copy the dependencies in case of a thin JAR. I feel this is too much JHipster is doing and it's getting complex even if it's being generated.

@SudharakaP may i suggest let's focus on the microservices side of thing first while we figure this out separately?

SudharakaP · 2019-10-10T21:26:32Z

@saturnism :

we shouldn't hard code Xmx here, since instances are different and heap size depends on the instance size. 😄

Agreed. But I was thinking we should set a default Xmx which is why I set it there. Anyhow I will revert that change since thinking about it again I think it isn't the most elegant.

@SudharakaP may i suggest let's focus on the microservices side of thing first while we figure this out separately?

Sure. 😄

saturnism · 2019-10-10T22:44:18Z

A reasonable default would need to be dynamic. I.e., F1 only has 256M of runtime memory. Setting -Xmx to 512MB will cause the app to exceed the runtime memory very quickly. Let's table this as I discuss more w/ Ludo and Appu.

SudharakaP · 2019-11-30T20:42:01Z

While working on #10345, I've come across some bugs and fixed them as well. 😄

Close/Open for tests

…Cloud SQL - this makes sure the correct port environment variable is passed whether the user chose to use cloud sql or not.

saturnism · 2019-12-11T19:49:33Z

@SudharakaP hiya! can this be merged for the initial support? we only supported monoliths at the beginning. microservices is harder, but we can discuss how to support that separately. wdyt?

SudharakaP · 2019-12-11T19:59:25Z

@saturnism: Hey, Yes of course it can be merged and I think it's better to be merged separately from any micro-service related logic. I didn't put any micro-service related code into it. I am working/thinking on the micro-service support. I think we are close, have to figure out some things related to the jhipster registry deployment and stuff. 😄

…

On Wed, Dec 11, 2019, 11:49 AM Ray Tsang ***@***.***> wrote: @SudharakaP <https://github.com/SudharakaP> hiya! can this be merged for the initial support? we only supported monoliths at the beginning. microservices is harder, but we can discuss how to support that separately. wdyt? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10420?email_source=notifications&email_token=AC64D7NF7MXOFBQKEJ25HLDQYE745A5CNFSM4IW3BXJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGULHHY#issuecomment-564704159>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC64D7KJUTW43E2XKRLT3Z3QYE745ANCNFSM4IW3BXJA> .

saturnism · 2019-12-11T20:54:48Z

could i trouble you to dm me on twitter @saturnism ? happy to discuss some of that over hangout.

in the mean time, i'll give this one more try and let's merge it soon. thnx!

SudharakaP · 2019-12-11T23:08:34Z

@saturnism : No problem. I didn't have twitter until recently. A few days back I created a twitter profile just to keep in loop with JHipster stuff. I am outside at the moment, but I'll dm you tonight for sure. 😄

pascalgrimaud · 2019-12-14T17:59:20Z

@SudharakaP : if I understand well, it's ready to be merged.
Unless someone else can test and review it, I'll trust you and merge it for the next release !

SudharakaP · 2019-12-14T18:07:59Z

@pascalgrimaud : Yes it's ready to be merged. @saturnism has indicated above that he'll give it one more try. 😄

pascalgrimaud · 2019-12-14T18:10:33Z

Yes I saw it, so I'll merge it just before the next release, so in 1 week maybe

SudharakaP · 2019-12-14T18:37:18Z

@pascalgrimaud : Yes, no problems; please feel free to 👍

saturnism

Found last 3 things. 2 in the comment, but 1 more is a verbiage change that I can't add to the comment here:

Initialize a new Cloud SQL instance (Y/N) ? -> Use a Cloud SQL instance (Y/N) ?

generators/gae/templates/application-prod-gae.yml.ejs

generators/gae/templates/pom-plugin.xml.ejs

SudharakaP · 2019-12-14T18:52:14Z

@saturnism : I've done the changes. Thanks much 😄

saturnism · 2019-12-14T19:16:38Z

/cc @PierreBesson @pascalgrimaud fyi - we think this is good to go! ready to be merged.

SudharakaP · 2019-12-16T03:00:26Z

@mraible : Thanks for merging 😄

SudharakaP · 2019-12-16T18:17:36Z

Bug bounty claimed at; https://opencollective.com/generator-jhipster/expenses/12444

Tonterias · 2020-02-05T20:37:59Z

I tried to use GAE in 6.6.0 but gave errors. Is it fixed in 6.6.0?

SudharakaP · 2020-02-05T21:12:10Z

@Tonterias : This ticket is for adding support for thin jars in GAE; which is completed in 6.6.0. If you see any specific errors, I would suggest opening a new issue with complete details or if it's a question use our gitter channel or stack overflow. 😄

SudharakaP and others added 11 commits September 14, 2019 11:35

add support for thin jar in gae

953917b

Only adds to maven build; work in progress

correcting mistake introduced in a pervious pull request

f1e0714

A previous pull request incorrectly removes the com.google.cloud.tools when gaeCloudSQLInstanceNeeded is set to false #10362

add correct handlers for retrieving static files

3de7627

remove bootWar from gradle

0176b66

The new GAE with Java 11 builds on JAR; this has been done on the maven side.

correcting mistake introduced in a pervious pull request

d9a91f4

A previous pull request incorrectly removes the com.google.cloud.tools when gaeCloudSQLInstanceNeeded is set to false

replace deprecated value "project" by "projectId"

bfbe0e8

Add copyright information

21fd518

add mainclass name instead of basename

6263295

add gradle support for unjaring the original jar

7fef90f

This adds Gradle side support for thin jars

missing symbol from previous commit

8df7380

SudharakaP changed the title ~~Add thin jar support gae~~ Add Support For Thin Jars in GAE Sep 15, 2019

saturnism reviewed Sep 16, 2019

View reviewed changes

generators/gae/templates/pom-plugin.xml.ejs Show resolved Hide resolved

remove the original jar as per discussion

de901fc

Merge branch 'master' into add-thin-jar-support-gae

db78a74

saturnism reviewed Sep 24, 2019

View reviewed changes

generators/gae/templates/app.yaml.ejs Outdated Show resolved Hide resolved

minor bug fix per code review

8b33fc1

SudharakaP mentioned this pull request Oct 1, 2019

jhipster gae seems to be broken #10533

Closed

add max heap size to entrypoint field in app.yaml

1ac6ec0

Revert Xmx change

fd742ff

SudharakaP closed this Nov 30, 2019

SudharakaP reopened this Nov 30, 2019

SudharakaP added 4 commits December 1, 2019 10:58

refactor code to include application-prod-gae.yml file regardless of …

994dc8e

…Cloud SQL - this makes sure the correct port environment variable is passed whether the user chose to use cloud sql or not.

fix dependency problem (similar to #9175) to get rid of the clean option

abcf0fa

Merge branch 'master' into add-thin-jar-support-gae

2f24b9c

add dependency before the check for cloud sql

f4874ba

SudharakaP mentioned this pull request Dec 11, 2019

JHipster Registry doesn't work with Google App Engine jhipster/jhipster-registry#400

Closed

1 task

Merge branch 'master' into add-thin-jar-support-gae

bc894d3

saturnism requested changes Dec 14, 2019

View reviewed changes

generators/gae/templates/application-prod-gae.yml.ejs Outdated Show resolved Hide resolved

generators/gae/templates/pom-plugin.xml.ejs Outdated Show resolved Hide resolved

changed per code review

22b80c1

saturnism approved these changes Dec 14, 2019

View reviewed changes

mraible merged commit 7d21a02 into jhipster:master Dec 16, 2019

SudharakaP deleted the add-thin-jar-support-gae branch December 16, 2019 03:00

pascalgrimaud added this to the 6.6.0 milestone Dec 20, 2019

SudharakaP mentioned this pull request Dec 26, 2019

Simplify Maven and Gradle Deployment Commands in GAE Subgenerator #11036

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Support For Thin Jars in GAE #10420

Add Support For Thin Jars in GAE #10420

SudharakaP commented Sep 15, 2019 •

edited

Loading

SudharakaP commented Sep 16, 2019

SudharakaP commented Sep 17, 2019 •

edited

Loading

saturnism commented Sep 19, 2019

SudharakaP commented Sep 19, 2019 •

edited

Loading

saturnism commented Sep 24, 2019

SudharakaP commented Sep 25, 2019

SudharakaP commented Oct 4, 2019

saturnism commented Oct 10, 2019

SudharakaP commented Oct 10, 2019 •

edited

Loading

saturnism commented Oct 10, 2019

SudharakaP commented Nov 30, 2019 •

edited

Loading

saturnism commented Dec 11, 2019

SudharakaP commented Dec 11, 2019 via email •

edited

Loading

saturnism commented Dec 11, 2019

SudharakaP commented Dec 11, 2019 •

edited

Loading

pascalgrimaud commented Dec 14, 2019

SudharakaP commented Dec 14, 2019

pascalgrimaud commented Dec 14, 2019

SudharakaP commented Dec 14, 2019 •

edited

Loading

saturnism left a comment

SudharakaP commented Dec 14, 2019

saturnism commented Dec 14, 2019 •

edited

Loading

SudharakaP commented Dec 16, 2019

SudharakaP commented Dec 16, 2019

Tonterias commented Feb 5, 2020

SudharakaP commented Feb 5, 2020

Add Support For Thin Jars in GAE #10420

Add Support For Thin Jars in GAE #10420

Conversation

SudharakaP commented Sep 15, 2019 • edited Loading

SudharakaP commented Sep 16, 2019

SudharakaP commented Sep 17, 2019 • edited Loading

saturnism commented Sep 19, 2019

SudharakaP commented Sep 19, 2019 • edited Loading

saturnism commented Sep 24, 2019

SudharakaP commented Sep 25, 2019

SudharakaP commented Oct 4, 2019

saturnism commented Oct 10, 2019

SudharakaP commented Oct 10, 2019 • edited Loading

saturnism commented Oct 10, 2019

SudharakaP commented Nov 30, 2019 • edited Loading

saturnism commented Dec 11, 2019

SudharakaP commented Dec 11, 2019 via email • edited Loading

saturnism commented Dec 11, 2019

SudharakaP commented Dec 11, 2019 • edited Loading

pascalgrimaud commented Dec 14, 2019

SudharakaP commented Dec 14, 2019

pascalgrimaud commented Dec 14, 2019

SudharakaP commented Dec 14, 2019 • edited Loading

saturnism left a comment

Choose a reason for hiding this comment

SudharakaP commented Dec 14, 2019

saturnism commented Dec 14, 2019 • edited Loading

SudharakaP commented Dec 16, 2019

SudharakaP commented Dec 16, 2019

Tonterias commented Feb 5, 2020

SudharakaP commented Feb 5, 2020

SudharakaP commented Sep 15, 2019 •

edited

Loading

SudharakaP commented Sep 17, 2019 •

edited

Loading

SudharakaP commented Sep 19, 2019 •

edited

Loading

SudharakaP commented Oct 10, 2019 •

edited

Loading

SudharakaP commented Nov 30, 2019 •

edited

Loading

SudharakaP commented Dec 11, 2019 via email •

edited

Loading

SudharakaP commented Dec 11, 2019 •

edited

Loading

SudharakaP commented Dec 14, 2019 •

edited

Loading

saturnism commented Dec 14, 2019 •

edited

Loading