You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@fineract.apache.org by Aleksandar Vidakovic <ch...@monkeysintown.com> on 2022/01/04 05:11:02 UTC

Recent improvemnts...

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 <pe...@gmail.com> 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.

Re: Recent improvemnts...

Posted by Aleksandar Vidakovic <ch...@monkeysintown.com>.
... just wanted to say: sorry for breaking the deployment... slipped my
radar... let me know if I can help in any way; I am following the
discussions on the issues you created.

:-)

On Wed, Jan 5, 2022 at 4:23 AM Michael Vorburger <mi...@vorburger.ch> wrote:

> This is incredible work!! Amazing contributions, Aleks.
>
> PS: FYI https://github.com/vorburger/www.fineract.dev/issues/11 re.
> related https://www.fineract.dev impact (will eventually fix it).
>
>
> On Tue, 4 Jan 2022, 06:11 Aleksandar Vidakovic, <ch...@monkeysintown.com>
> 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 <pe...@gmail.com> 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.
>>
>

Re: Recent improvemnts...

Posted by Aleksandar Vidakovic <ch...@monkeysintown.com>.
... right... good catch! I'll take care of this as soon as possible.

+1

On Wed, Jan 5, 2022 at 11:28 PM Petri Tuomola <pe...@gmail.com>
wrote:

> 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 <mi...@vorburger.ch> wrote:
>
> This is incredible work!! Amazing contributions, Aleks.
>
> PS: FYI https://github.com/vorburger/www.fineract.dev/issues/11 re.
> related https://www.fineract.dev impact (will eventually fix it).
>
>
> On Tue, 4 Jan 2022, 06:11 Aleksandar Vidakovic, <ch...@monkeysintown.com>
> 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 <pe...@gmail.com> 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.
>>
>
>

Re: Recent improvemnts...

Posted by Petri Tuomola <pe...@gmail.com>.
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 <mi...@vorburger.ch> 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, <cheetah@monkeysintown.com <ma...@monkeysintown.com>> 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 <ma...@gmail.com> 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.


Re: Recent improvemnts...

Posted by Michael Vorburger <mi...@vorburger.ch>.
This is incredible work!! Amazing contributions, Aleks.

PS: FYI https://github.com/vorburger/www.fineract.dev/issues/11 re. related
https://www.fineract.dev impact (will eventually fix it).


On Tue, 4 Jan 2022, 06:11 Aleksandar Vidakovic, <ch...@monkeysintown.com>
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 <pe...@gmail.com> 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.
>

Re: Recent improvemnts...

Posted by Aleksandar Vidakovic <ch...@monkeysintown.com>.
Hi everyone,

... quick status update on the integration tests: I've reduced the previous
failures from > 60 to 17. The remaining failures are almost certainly due
to concurrency issues.

As for the speed: looks like we are pinned on 2h running with Github
Actions.

FYI

On Tue, Jan 4, 2022 at 6:11 AM Aleksandar Vidakovic <
cheetah@monkeysintown.com> 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 <pe...@gmail.com> 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.
>