Petri, I thought about this thread again while I was looking into https://issues.apache.org/jira/browse/FINERACT-914 today, and regretted that we still use a standalone Tomcat and not one Spring can embed... ;-) So in order to support this to happen, here is a lengthier more technical reply to your (great!) questions inline:
On Fri, May 1, 2020 at 2:15 PM Awasum Yannick <[email protected]> wrote: > Hi Petri > > Thanks for doing all this work. Its invaluable. > > On Fri, May 1, 2020 at 10:24 AM Petri Tuomola <[email protected]> wrote: > >> Great stuff! >> >> Having done some testing on this, what seems to work is: >> >> - bootRun starts Spring Boot embedded Tomcat and works as expected >> >> - bootJar creates a JAR that can be executed from command line and works >> as expected >> >> - bootWar creates a WAR that can be deployed to an external Tomcat and >> works as expected. However the WAR file can’t be started directly from >> command line, as OpenJPA doesn’t seem to be able to find >> persistence-enhanced classes from within a WAR (looking at the source, it >> seems to be hardcoded to only handle classes from filesystem or JAR). >> > Ah, interesting, I see. I guess in theory there is probably a way to fix that, but I would just ignore it, it's super low priority; knowing that the JAR works. One thing we can do is just point this out in future documentation on the README. I wouldn't worry about it. > What doesn’t seem to work anymore after my changes is the “other” embedded >> Tomcat - i.e. version 7.0.94 - that is included directly as a dependency >> in build.gradle and used for tasks tomcatRunWar / tomcatRun. I suppose >> that’s not surprising as there are two embedded Tomcats bundled that are >> likely to conflict, so getting both to work will require some thinking. >> >> To the best of my knowledge the tomcatRunWar task is used in development > for quickly testing changes one makes as they solve a problem. Is it also > used for integration tests? It might be Ok to just forget about this and > use the "newly fixed" (PR still to be sent) bootRun Spring Boot gradle > tasks. > I think I see the problem you are facing. I totally agree with what Awasum as expressed. Basically, don't be shy to yank out things from the past to help this project get to tomorrow! ;-) Perhaps if we formulate the following "requirements", this may help you to zoom into the best solution: A. We want to be able to easily run Fineract during development. Whether that is by "tomcatRunWAR" as today or "bootRun" tomorrow doesn't matter at all, if they are basically fully equivalent for a developer. bootRun is obviously preferably - especially it can hot reload changes... ;-) Just remember to also update the documentation in the README in the PR, if you can. B. We do need those integration tests to run somehow - but we don't care at all how they run... :) Today, this works like this: The integrationTest Gradle task, see https://github.com/apache/fineract/blob/develop/fineract-provider/build.gradle#L495, uses the tomcatRunWarAsDaemonForIntegrationTest Gradle task, which is based on https://github.com/bmuschko/gradle-tomcat-plugin. I think you have basically two ways forward here, you either: 1. go all the way in and do https://issues.apache.org/jira/browse/FINERACT-764 together with https://issues.apache.org/jira/browse/FINERACT-730 right away, meaning we'd run integration tests through Spring's support for such things, instead of starting an external Tomcat through Gradle, and totally remove the "other" (original, old) Tomcat dependencies and the gradle-tomcat-plugin, as in this scenario they won't be needed anymore. 2. stick, for now, to having a Tomcat launched for the ITs - but that won't, yet, be the "new" Tomcat which you'll embed for Spring Boot. I think what's important to realize here is that, as long as (quoting you) "bootWar creates a WAR that can be deployed to an external Tomcat and works as expected", that should still be totally possible, do you agree? I suspect the real problem you are facing is a classical classpath mess between the two Tomcats. In theory, if https://github.com/bmuschko/gradle-tomcat-plugin were to do classpath management well, this hypothetically should work (with different classloaders), but if you find that it doesn't, then just forget about that - and let's just find another way? I noticed on https://github.com/bmuschko/gradle-tomcat-plugin that, quote "Gradle starts the embedded container in the same JVM. Currently, the container cannot be forked as a separate process. If you are looking for this capability, please have a look at the Cargo plugin instead." and had a quick look at https://github.com/bmuschko/gradle-cargo-plugin. I think if we just switched to using that gradle-cargo-plugin instead of the gradle-tomcat-plugin for the integration tests, for now, that would solve the problem you're hitting? It would likely be a little bit slower, as that approach will require the full WAR to be built, and deployed into a separately running Tomcat in another JVM (which is exactly what the gradle-cargo-plugin must be doing...), but given how slow our integration tests overall run, that's the least of our worries... ;-) Is that approach something you would like to try? I guess in theory it could even be done separately by someone else as a pre-requisite... anyone reading this up for that? I suspect 1. is more work, and because we seem to finally be almost there with 2. my recommendation would be to first go that way (2.), get that merged, and only then dig into 1, as a fully separate next follow-up step. As for tomcatRunWar VS tomcatRun, see https://github.com/apache/fineract/pull/795, which proposes we just remove tomcatRun! (I briefly tried it, but got a [strange!] error, and anyway don't quite see the point of the tomcatRun gradle task, as opposed to tomcatRunWar; Caused by: java.lang.AbstractMethodError: Receiver class com.zaxxer.hikari.metrics.micrometer.MicrometerMetricsTrackerFactory does not define or inherit an implementation of the resolved method 'abstract com.zaxxer.hikari.metrics.MetricsTracker create(java.lang.String, com.zaxxer.hikari.metrics.PoolStats)' of interface com.zaxxer.hikari.metrics.MetricsTrackerFactory.) One thing you can do is run the integrationTest tasks on the commandline > and see if everything passes on your local Dev machine as that may use > tomcatRunWar. > > I dont know if anyone has an Objection to that. Either way, Send your PR > so we can also test while reviewing the code. Its not a problem as we can > revert if we find that disable the tomcatRunWar tasks breaks something > after merging. > > >> Before I try to find a way to get both of these to work, I wanted to >> check if these are both needed? I.e. is there a reason to include two >> different embedded Tomcat versions and two Gradle tasks to start them? Or >> would it be possible to just remove the older version and related Gradle >> tasks & plugin, and use the Spring Boot version only? >> > > Lets use the bootRun tasks for dev purposes except someone objects. Send > your PR Petri > > Thanks. > Awasum > > > On 30 Apr 2020, at 11:11 PM, Michael Vorburger <[email protected]> wrote: >> >> Petri, >> >> welcome to Apache Fineract. >> >> Yes, a PR for this would be *VERY* welcome... ;-) >> >> Tx, >> M. >> >> >> On Thu, 30 Apr 2020, 22:07 Petri Tuomola, <[email protected]> >> wrote: >> >>> Hi all >>> >>> I’m new to Fineract - so apologies in advance for anything I may have >>> got wrong with this: >>> >>> I had a bit of spare time today and downloaded the latest develop branch >>> for Fineract 1.x. Initially it did not start on my machine - ./gradlew >>> bootRun threw a lot of errors. >>> >>> But after a couple of fixes to dependencies, SSL config, main class etc, >>> ./gradlew bootRun now seems to work fine: the application starts and I can >>> call the API etc. >>> >>> My changes are in a branch at >>> https://github.com/ptuomola/fineract/tree/FINERACT-730 and I was >>> wondering if these would be of interest to anyone? >>> >>> I think this should help to address the issue FINERACT-730 >>> >>> If yes, I can open a pull request - once I’ve done some more testing >>> (e.g. that the built WAR can also be started and that I haven’t broken the >>> dev profile build etc) >>> >>> Thanks in advance >>> >>> Regards >>> Petri >>> >>
