Hi Aleks

One suggestion on the automatic build script version management (which I think 
is a great improvement btw):

We now have two different version numbers generated automatically - one in the 
build script by jgitver, and another one by generateGitProperties

The bad thing is that currently the version numbers generated by these do not 
match. 

For example, trying this out now: 

- With jgitver, I get "1.5.1-b338ccc1”. This is used e.g. to name the JAR file
- With generatedGitProperties, I get "1.5.0-264-gb338ccc” This is used e.g. in 
the Swagger-UI as a version number

Finally, /actuator/info actually shows both of the version numbers!

Should we standardise this and just use one across the board? Will be a bit 
confusing if we have both…

Cheers
Petri


> On 5 Jan 2022, at 11:21, Michael Vorburger <[email protected]> wrote:
> 
> This is incredible work!! Amazing contributions, Aleks.
> 
> PS: FYI https://github.com/vorburger/www.fineract.dev/issues/11 
> <https://github.com/vorburger/www.fineract.dev/issues/11> re. related 
> https://www.fineract.dev <https://www.fineract.dev/> impact (will eventually 
> fix it).
> 
> 
> On Tue, 4 Jan 2022, 06:11 Aleksandar Vidakovic, <[email protected] 
> <mailto:[email protected]>> wrote:
> Hi everyone,
> 
> ... just wanted to give you a bit of an update on some recent improvements 
> that have an impact on build speed and development experience:
> 
> 
> OpenJPA plugin
> 
> Petri spent some time getting the OpenJPA plugin to do the enhancement 
> processing in a way to take full advantage of the build cache. This one is 
> actually a major speed improvement overall as it allows now for truly 
> incremental compilation of "fineract-provider" (you'll see, the first compile 
> takes quite some time... there's a lot of code, but the second one just skips 
> when no changes to the sources were made.
> 
> 
> Fix Gradle compiler parameters
> 
> This was an odd one. Turns out if you add compiler parameters multiple times 
> then this slows down the build process significantly (I have no explanation, 
> just experimentation). That one is on me, I introduced this a while back, 
> because I didn't see that we can add all parameters at once. Low hanging 
> fruit.
> 
> 
> Automatic build script version management based on Git tags and Git hashes
> 
> This one is a little bit of a convenience. Until now we manually set (or not) 
> a property ("releaseVersion") in gradle.properties to determine the version 
> of Fineract. This is easy to forget when doing the releases. Now that we have 
> Docker builds based on a Gradle plugin this also comes in handy when setting 
> the Docker tags (just use the "$version" variable). The only thing we need to 
> do now are proper releases with Git tags. Git hashes will be part of the 
> version number during development (pull requests, develop branch); when we do 
> a release (in master) the version pattern automatically follows the Semantic 
> Versioning rules (no Git hash). BTW: Git hashes change on every commit; like 
> this all important artifacts become uniquely identifiable no matter which 
> branch you are working in.
> 
> 
> Create separate sub-project for WAR build
> 
> We have now a separate module for the WAR build. This was done in preparation 
> to improve the integration tests, but also to simplify the 
> "fineract-provider" build script (was needed for the Docker build 
> improvement). 
> Note: personally I would drop this deliverable sooner than later; with a 
> proper Docker image build in place this shouldn't be needed anymore. We used 
> it here mainly for setting up tests (see my comments below concerning 
> "Testcontainers"). Is there a significant audience that still uses these WAR 
> files in production?
> 
> 
> Docker image build with Gradle Jib plugin
> 
> I'm pretty sure up until now the Docker image build via the 
> docker-compose.yml file was slowing down pretty much everyone. On my machine 
> this took > 10min, because the Dockerfile contained one section that did a 
> Gradle build inside a Docker container and the results of that build were 
> then used to populate the actual Docker image. Because everything ran in a 
> Docker container that build could not take advantage of any caches outside of 
> it where we did pretty much the same. So, instead of having to build twice we 
> now only have one run that takes advantage of Gradle's build cache and the 
> Docker image is built now by Google's Jib plugin which is very fast (just a 
> couple of secs) and does some optimizations to keep the images small.
> 
> 
> Replace Tomcat Cargo plugin to run integration tests with Testcontainers
> 
> This is a major improvement for the development experience. Until now it was 
> a bit complicated to run the integration tests locally. You needed a separate 
> MySQL instance running that required some initialization (see Gradle script 
> "createDb" tasks). This also made it a bit inconvenient to run those tests in 
> your IDE. Not anymore! Now that we have a Docker image as part of the build 
> process we re-use it immediately for those integration tests. The 
> Testcontainers  framework brings Docker to JUnit 5 tests with not much 
> effort. Actually, the only changes I applied to the existing tests were to 
> make sure that the proper ports are used (Testcontainers uses random ports 
> which allows you to run - in theory - multiple Fineract instances side by 
> side) and in some cases I needed to enforce a certain execution order of test 
> functions, because they had concurrency issues. Looks like not all tests are 
> thread safe. Which reminds me that I've tentatively enabled parallel test 
> execution to speed up things.
> I also got rid of the separate modules for OAuth2 and 2FA integration 
> testing; this can now be done in the same module. Simpler Github Actions 
> workflow.
> Note: this PR is still in progress, but looks already promising. More changes 
> on the testing front coming (Cucumber, faster JUnit tests with minimal or no 
> Docker container dependencies)
> 
> 
> Simplify selection of auth strategies
> 
> As you know we decided on build time which authentication scheme to use. 
> Additionally you had to use certain command line parameters to activate 
> related Spring profiles. This was a bit redundant, especially since we had 4 
> different "application.properties" files that we copied into the 
> "fineract-provider/src/main/resources" folder. This was one of the main 
> reasons why we constantly did a full rebuild; "src/main/resources" is 
> monitored by Gradle for changes... and obviously if we copy a file over and 
> over again into that folder (which changes its timestamp even if there are no 
> changes) then Gradle assumes we have a change and triggers the build. There 
> is still some room for improvement, but it behaves already a lot nicer now.
> 
> 
> Upgrade to JDK 17
> 
> JDK 17 is already out for a while and it's an LTS release, so we upgraded 
> everything (Gradle build script, Docker image, Fineract) to be compatible 
> with it. Changes were mostly command line parameters to allow access to 
> certain JDK modules. There was one interesting change necessary concerning 
> "GsonBuilder"; we were instantiating that class in quite a lot of places 
> (mostly those manual mapper classes that we use to map JSON to Java classes). 
> The problem with just calling "new GsonBuilder()" was that this doesn't 
> automatically register all necessary Gson serializers (in this case JDK 17 
> complained that the serializer for "java.time.LocalTime" was missing). We 
> have now a static function "GoogleGsonSerializerHelper.createGsonBuilder()" 
> that ensures that all these serializers are properly registered.
> There was one other rather curious issue related to date formats. In one of 
> the tests we had a date string "15 Sep 2015" (and others like "15 March 2015" 
> etc.); but this particular one for September always failed in JDK 17 with the 
> date pattern "dd MMMM yyyy" applied. This September date is not accepted 
> anymore in JDK 17. The correct value is now "15 September 2015". This kind of 
> makes sense as "Sep" is an abbreviation and the other months are 
> formatted/parsed with full month names; someone must have cleaned up this 
> behavior in the latest JDK release.
> 
> 
> Coming soon...
> Introduce JReleaser: JReleaser includes a lot of best practices for creating 
> releases; I'll replace my home grown script (and the somewhat lengthy 
> description on how to run it) with this plugin; should make life a lot easier.
> More Github Actions Docker build cache improvements: I'll try to preserve 
> some of the already downloaded Docker layers to improve the overall build 
> speed
> More build speed improvements: at the moment the fineract-client module is 
> still triggering a full rebuild too often; similar situation with the Docker 
> image build plugin. Both tasks don't take that long, but still would be a 
> nice tweak.
> Removing implicit dependencies: we have quite some places where we explicitly 
> force the execution of tasks to make sure that everything is executed in the 
> proper order; this doesn't really work very well to get the most out of 
> caching. Removing these dependencies should speed up builds quite a lot.
> 
> I think that's it more or less for this round. Thanks again to @Petri Tuomola 
> <mailto:[email protected]> for reviewing all these changes and 
> contributing the OpenJPA plugin improvements.
> 
> README contains more detailed information on some of these changes. Please 
> let me know if something is missing.
> 
> FYI
> 
> Cheers,
> 
> Aleks
> 
> P.S.: ... in the meanwhile I did a full integration test run locally on my 
> machine, just to quickly check where we are. Parallel execution would take 
> probably only half the time (around 30-40min), but at the moment there are 
> just too many concurrency issues so reverted back to sequential test 
> execution. The current execution time on my machine for all integration tests 
> is > 2h with 530 tests executed of which 27 failed (still running... 
> "ClientLoanIntegrationTest" is quite a monster). Not too bad for a first 
> shot; currently trying to make it faster while trying to keep the tests as 
> they are.

Reply via email to