You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/05/04 21:22:17 UTC
[GitHub] [fineract] ptuomola opened a new pull request #820: FINERACT-846: Migrating to Java 11
ptuomola opened a new pull request #820:
URL: https://github.com/apache/fineract/pull/820
## Description
Made changes to Gradle configuration and dependencies to migrate Fineract to Java 11
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [X] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
- [X] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
- [ ] API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.
- [X] Integration tests have been created/updated for verifying the changes made.
- [X] All Integrations tests are passing with the new commits.
- [X] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on a change in pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
xurror commented on a change in pull request #820:
URL: https://github.com/apache/fineract/pull/820#discussion_r421250304
##########
File path: fineract-provider/build.gradle
##########
@@ -21,6 +21,9 @@ Run as:
gradle clean bootRun'''
project.ext.jerseyVersion = '1.19.4'
+ext["rest-assured.version"] = '4.3.0'
Review comment:
Yeah, about that. spring dependency has an option to define a set of dependencies(dependencySet) for libraries which have thesame version number for different artifacts. There are samples of that in https://github.com/apache/fineract/pull/662. So you should be able to easily get the gist once you look that. With that you don't have to define dependencies jar-by-jar. Do you get what I mean here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
xurror commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-625027942
Aside from that, this is really a good job. We might have to do something about the java version in travis.yml.
@vorburger please can you have a look at this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #820: FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-623924104
Integration Tests are failing. Maybe run them locally and take time to see if you can fix them. This is valuable work. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #820: FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-624109613
Thanks everyone for your comments. This is still a bit WIP - I've just fixed the depreciated and unchecked warnings, as well as some other build errors. Still a few integration tests failing - will fix them next.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #820: FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-623889493
Hello @ptuomola . Thanks for taking on this work. You forgot to change the JDK version to 11 in Travis.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on a change in pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #820:
URL: https://github.com/apache/fineract/pull/820#discussion_r421244604
##########
File path: fineract-provider/build.gradle
##########
@@ -21,6 +21,9 @@ Run as:
gradle clean bootRun'''
project.ext.jerseyVersion = '1.19.4'
+ext["rest-assured.version"] = '4.3.0'
Review comment:
The reason why I did it this was that with a single line you can ensure that all the rest-assured libraries are brought in with the right version. I was first doing it through dependencies, but that seemed to require separately setting the version for every single rest-assured jar. That is of course much more error-prone: if you miss one, suddenly you are in classpath hell again. But if that's preferred I can of course set it jar-by-jar...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-625009848
@xurror @awasum I found the SSL issue (bug in OpenJDK) and all the tests now seem to pass, so this should be ready for your review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #820: FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-623924990
Seems to be very similar problems for all the test. Looks like when one is fixed..you can easily copy to the other failing tests and fix them too.
` java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202
org.apache.fineract.integrationtests.RecurringDepositTest > testRecurringDepositAccountWithAmountInterestRateChart_PERIOD_VARIATION FAILED
java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202
org.apache.fineract.integrationtests.RecurringDepositTest > testRecurringDepositAccountWithClosureTypeTransferToSavings_WITH_HOLD_TAX FAILED
java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202
org.apache.fineract.integrationtests.RecurringDepositTest > testRecurringDepositAccountUndoApproval FAILED
java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202
org.apache.fineract.integrationtests.RecurringDepositTest > testMaturityAmountForDailyCompoundingAndMonthlyPosting_With_360_Days FAILED
java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202
org.apache.fineract.integrationtests.RecurringDepositTest > testRecurringDepositAccountWithAmountAndPeriodInterestRateChart_AMOUNT_VARIATION FAILED
java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202
org.apache.fineract.integrationtests.RecurringDepositTest > testRecurringDepositAccountWithPeriodInterestRateChart_PERIOD_VARIATION FAILED
java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202
org.apache.fineract.integrationtests.RecurringDepositTest > testMaturityAmountForDailyCompoundingAndMonthlyPosting_With_365_Days FAILED
java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202
org.apache.fineract.integrationtests.RecurringDepositTest > testPrematureClosureAmountWithPenalInterestForWholeTerm_With_360_Days FAILED
java.lang.RuntimeException at RecurringDepositTest.java:109
Caused by: javax.net.ssl.SSLPeerUnverifiedException at RecurringDepositTest.java:109
java.lang.NullPointerException at RecurringDepositTest.java:3202`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-624579121
This is still work in progress - but looked like if I kept it as draft then Travis would not build it. Will let you know once I've fixed the integration tests and then it's ready for review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-625132396
This is another major change for the project. So a mailing list message
about this will be great. Thank you all.
On Thu, May 7, 2020, 09:56 Michael Vorburger ⛑️ <no...@github.com>
wrote:
> @ptuomola <https://github.com/ptuomola> wow, great job, nice find re.
> JDK!!! I'm just curious, did you notice this by comparison of Travis's VS
> local JDK version numbers, or do you have a link to a OpenJDK bug about
> this?
>
> Using JDK 12 instead of 11 can't stay long, because even though Gradle
> specifies 11, that will only cause javac to allow only 11 SYNTAX, but it
> will still allow to build with use of 12 LIBRARIES - I've dealt with
> related problems in other projects, over the years. But what I think we
> should do due to OpenJDK bug is still do this, for now, merge, but then
> change it "soon". I volunteer to look into it for Travis later. Perhaps
> create a new JIRA dedicated to "Switch Travis from Java 12 to Java 11" (and
> assign it to me)?
>
> As for javac compiler warnings, let's fix in follow up PRs. Personally I
> may even have left the warning instead of suppressing, as a reminder, but
> it really doesn't matter, ignore.
>
> I was also going to suggest that this could have been broken up into
> separate PRs first for all the code changes, while still on Java 8, and
> then figure out make the Java 11 build changes, but seeing your
> breakthrough, I'll get this in ASAP; so more of a suggestion for future
> smaller PRs.
>
> Let me fully review this by staring at the diff a little longer after work
> today, and likely merge ASAP.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/fineract/pull/820#issuecomment-625123674>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ADWPAYDM3NF647N6HBUWJXDRQJZTPANCNFSM4MZDAOXA>
> .
>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on a change in pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
xurror commented on a change in pull request #820:
URL: https://github.com/apache/fineract/pull/820#discussion_r421235125
##########
File path: fineract-provider/dependencies.gradle
##########
@@ -80,9 +80,10 @@ dependencies {
'org.drizzle.jdbc:drizzle-jdbc',
- 'org.apache.poi:poi',
- 'org.apache.poi:poi-ooxml',
- 'org.apache.poi:poi-ooxml-schemas',
+ 'org.apache.poi:poi:4.1.2',
Review comment:
Since https://github.com/apache/fineract/pull/662, we don't want to set version numbers for prject dependencies in dependencies.gradle but in the build.gradle.
Version numbers don't really fit here anymore. Look at https://github.com/apache/fineract/pull/662 for more about this.
##########
File path: fineract-provider/build.gradle
##########
@@ -21,6 +21,9 @@ Run as:
gradle clean bootRun'''
project.ext.jerseyVersion = '1.19.4'
+ext["rest-assured.version"] = '4.3.0'
Review comment:
Also, I don't think this will be necessary since you can just define the dependency as is done https://github.com/apache/fineract/pull/662
##########
File path: fineract-provider/dependencies.gradle
##########
@@ -80,9 +80,10 @@ dependencies {
'org.drizzle.jdbc:drizzle-jdbc',
- 'org.apache.poi:poi',
- 'org.apache.poi:poi-ooxml',
- 'org.apache.poi:poi-ooxml-schemas',
+ 'org.apache.poi:poi:4.1.2',
+ 'org.apache.poi:poi-ooxml:4.1.2',
Review comment:
I also noticed some of these dependencies were already defined in build.gradle though just older, so you could just go and update the version numbers there. Is that OK with you??
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on a change in pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #820:
URL: https://github.com/apache/fineract/pull/820#discussion_r421244693
##########
File path: fineract-provider/dependencies.gradle
##########
@@ -80,9 +80,10 @@ dependencies {
'org.drizzle.jdbc:drizzle-jdbc',
- 'org.apache.poi:poi',
- 'org.apache.poi:poi-ooxml',
- 'org.apache.poi:poi-ooxml-schemas',
+ 'org.apache.poi:poi:4.1.2',
Review comment:
OK - will have a look to see if there are duplicates, and fix that. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on a change in pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #820:
URL: https://github.com/apache/fineract/pull/820#discussion_r421254618
##########
File path: fineract-provider/build.gradle
##########
@@ -21,6 +21,9 @@ Run as:
gradle clean bootRun'''
project.ext.jerseyVersion = '1.19.4'
+ext["rest-assured.version"] = '4.3.0'
Review comment:
Sorry - I thought the dependencySet only set the version for the JARs you've liste in the set, but not for the dependencies that those dependencies bring in (i.e. the transitive dependencies).
But if it works for the non-listed transitive dependencies as well, then of course that will have the same result as this. I'll try it out and change if it works.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on a change in pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #820:
URL: https://github.com/apache/fineract/pull/820#discussion_r421271515
##########
File path: fineract-provider/build.gradle
##########
@@ -21,6 +21,9 @@ Run as:
gradle clean bootRun'''
project.ext.jerseyVersion = '1.19.4'
+ext["rest-assured.version"] = '4.3.0'
Review comment:
Looks like it does not work for transitive dependencies - you need to explicitly list all the jars for which you want the version to apply. I've done this and listed the JARs we are currently using. But of course if we add some in the future then this list needs to be updated to avoid incompatible classes...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on a change in pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #820:
URL: https://github.com/apache/fineract/pull/820#discussion_r421254618
##########
File path: fineract-provider/build.gradle
##########
@@ -21,6 +21,9 @@ Run as:
gradle clean bootRun'''
project.ext.jerseyVersion = '1.19.4'
+ext["rest-assured.version"] = '4.3.0'
Review comment:
Sorry - I thought the dependencySet only set the version for the JARs you've liste in the set, but not for the dependencies that those dependencies bring in (i.e. the transitive dependencies).
But if it works automatically for the non-listed transitive dependencies as well, then of course that will have the same result as this. I'll try it out and change if it works.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola edited a comment on pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola edited a comment on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-625004656
> @ptuomola I've run into this kind of thing before - it could be caused e.g. by Oracle Java vs. OpenJDK, which include different root certificates. Haven't investigated this particular problem, but thought I'd mention it here in case that is it.
@vorburger Thanks! Found the issue: there is a bug in OpenJDK 11.0.2. They've implemented SSL connection reuse, but the reuse discards all the certificates shared during the earlier use of the connection, which results in this error. relaxedHTTPSValidation doesn't help as it still expects some certificate, and in this case there is none.
The issue is fixed in later OpenJDK 11 builds (e.g. 11.0.7) which is what I have on my own machine, and hence I did not see this. However Travis pulls in 11.0.2 and I was not able to find an easy way to get it to use a later minor version.
So I fixed the Travis build by getting it to use OpenJDK 12 which has this fix. I'm assuming that will still be OK, because in the Gradle file we specify JDK 11. Alternatively need to find some way to get Travis to download and install 11.0.7 instead...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #820: FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-625167037
Many thanks @vorburger for your feedback and comments
> @ptuomola wow, great job, nice find re. JDK!!! I'm just curious, did you notice this by comparison of Travis's VS local JDK version numbers, or do you have a link to a OpenJDK bug about this?
I found it by turning on the SSL debug logging and stepping through the SSL handshakes / negotiation. But once I worked out what's going on, it was easy to find a JDK bug that someone else had raised on the same issue and that has been fixed in a later JDK minor version (https://bugs.openjdk.java.net/browse/JDK-8212885 - TLS 1.3 resumed session does not retain peer certificate chain)
> Using JDK 12 instead of 11 can't stay long, because even though Gradle specifies 11, that will only cause javac to allow only 11 SYNTAX, but it will still allow to build with use of 12 LIBRARIES - I've dealt with related problems in other projects, over the years. But what I think we should do due to OpenJDK bug is still do this, for now, merge, but then change it "soon". I volunteer to look into it for Travis later. Perhaps create a new JIRA dedicated to "Switch Travis from Java 12 to Java 11" (and assign it to me)?
That makes sense. There are some possible ways suggested on how to get a specific version of OpenJDK on different sites - e.g. https://stackoverflow.com/questions/29636754/can-you-specify-minor-jdk-version-for-travis-ci. However none of them seemed particularly "clean".
I also considered upgrading the whole Linux distro being used by Travis to a newer version (probably a good idea anyway). But unfortunately the newer distro version still uses the same script to pull OpenJDK, and hence seemed to end up with the same JDK version.
> As for javac compiler warnings, let's fix in follow up PRs. Personally I may even have left the warning instead of suppressing, as a reminder, but it really doesn't matter, ignore.
I've left this for now.
> I was also going to suggest that this could have been broken up into separate PRs first for all the code changes, while still on Java 8, and then figure out make the Java 11 build changes, but seeing your breakthrough, I'll get this in ASAP; so more of a suggestion for future smaller PRs.
Thanks for the suggestion - will in the future submit smaller PRs. There are still a couple of cleanups I think would make sense - e.g. there's still a depreciation warning from Gradle, and some of the Gradle plugins are not at their latest versions - but I can submit those as separate PRs.
Thanks again
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
xurror commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-624587954
It's OK, just ping any of us when you think it's ready for review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #820: FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-624271467
@xurror @awasum Interestingly I don't get the same failures with integration tests when I run them on my machine locally.
The error seems to caused by the test client not trusting the HTTPS certificate provided by the server. But that should never happen as we are using relaxedHTTPSValidation() from RestAssured, which should skip all of the certificate validations. And on my local machine that seems to work as expected.
Anyway - will investigate further tomorrow, maybe I can find a way to reproduce the error locally.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-625123674
@ptuomola wow, great job, nice find re. JDK!!! I'm just curious, did you notice this by comparison of Travis's VS local JDK version numbers, or do you have a link to a OpenJDK bug about this?
Using JDK 12 instead of 11 can't stay long, because even though Gradle specifies 11, that will only cause javac to allow only 11 SYNTAX, but it will still allow to build with use of 12 LIBRARIES - I've dealt with related problems in other projects, over the years. But what I think we should do due to OpenJDK bug is still do this, for now, merge, but then change it "soon". I volunteer to look into it for Travis later. Perhaps create a new JIRA dedicated to "Switch Travis from Java 12 to Java 11" (and assign it to me)?
As for javac compiler warnings, let's fix in follow up PRs. Personally I may even have left the warning instead of suppressing, as a reminder, but it really doesn't matter, ignore.
I was also going to suggest that this could have been broken up into separate PRs first for all the code changes, while still on Java 8, and then figure out make the Java 11 build changes, but seeing your breakthrough, I'll get this in ASAP; so more of a suggestion for future smaller PRs.
Let me fully review this by staring at the diff a little longer after work today, and likely merge ASAP.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #820: [WIP] FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-625004656
> @ptuomola I've run into this kind of thing before - it could be caused e.g. by Oracle Java vs. OpenJDK, which include different root certificates. Haven't investigated this particular problem, but thought I'd mention it here in case that is it.
Thanks! Found the issue: there is a bug in OpenJDK 11.0.2. The logic for reusing a SSL connection discards all the certificates shared during the earlier use of the connection, which results in this error. relaxedHTTPSValidation doesn't help as it still expects some certificate, and in this case there is none.
The issue is fixed in later OpenJDK 11 builds (e.g. 11.0.7) which is what I have on my own machine, and hence I did not see this. However Travis pulls in 11.0.2 and I was not able to find an easy way to get it to use a later minor version. So I fixed the Travis build by getting it to use OpenJDK 12 which has this fix. I'm assuming that will still be OK, because in the Gradle file we specify JDK 11. Alternatively need to find some way to get Travis to download and install 11.0.7 instead...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #820: FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-624275571
@ptuomola I've run into this kind of thing before - it could be caused e.g. by Oracle Java vs. OpenJDK, which include different root certificates. Haven't investigated this particular problem, but thought I'd mention it here in case that is it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #820: FINERACT-846: Migrating to Java 11
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #820:
URL: https://github.com/apache/fineract/pull/820#issuecomment-625485682
FINERACT-937 created re Java 12 -> 11 on Travis.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org