FINERACT-982 - Completely ditch use of Drizzle JDBC Driver #1366
Conversation
|
I won't bet on this passing the build. I ran tests locally 3 times but some schedular jobs test kept failing. I guess schedular jobs will just do their thing. |
|
I would externalize all references to specific drivers. What I mean with that is: instead of hard-coding the driver's class names and connection strings I would replace it with environment variables. Defaults for those variables would be the Drizzle related settings (for license compatibility), but anyone running Fineract in production (I guess that's the concern here) would then overwrite those environment variables either in the docker-compose.yml file or on the command line. That way no tricks have to be applied during releases and we don't have to reference the LGPL driver in this repository... which - as much as I understand it - we actually can't (LGPL being incompatible with Apache License 2)... unless I'm missing anything here? |
|
@vidakovic I think we also have a challenge using the current config in development as it causes challenges during testing. You can follow discussion here on why these changes are the way they are: https://issues.apache.org/jira/browse/FINERACT-982 https://issues.apache.org/jira/browse/FINERACT-980. FYI: @vorburger @ptuomola @awasum |
|
In the end there are various mechanisms that we could use here to set different properties (e. g. Spring profiles)... agreed, tests would need maybe a bit of thinking... The one thing that seems to me unsolvable (disclaimer: I'm not a lawyer) is the dependency reference to the LGPL jar. And if we can't include that then the devs have to do manual fiddling anyway. Easiest in terms of contributors would be (but these are more long-term - if it all - solutions that are more intrusive):
Just spitballing... |
|
Adopting profiles was my first option too. Perhaps we could get more inputs how to go about this from the community. |
|
👍 |
|
Ok, I think I have a bit more info how other Apache projects deal with the issue of (L)GPL licensed libraries. See e. g. https://github.com/apache/camel/blob/master/components/camel-debezium-mysql/pom.xml. The important part to note here is the dependency scope ("provided") which means they don't include the dependency in the final build artifact. As far as I know the closest equivalent in Gradle should be compileOnly. That means that at least from the IDE perspective this issue should be solved (e. g. IntelliJ doesn't make a distinction between these scopes). Not sure if that applies also for running unit tests on the console, would need to test. It will not be included in the release package or Docker images (automatically). The current Dockerfile does download mysql-connector-j from Maven Central with a wget command and the jar file is then included to the Fineract startup with "java -cp"... I guess that means the Docker images are not considered release artifacts. Just FYI |
|
Nice digging. I'll try it out. |
| PRE-RELEASE NOTICE: Please before starting any release process, ensure that the you have replaced mysql connector package from mariadb connector/j back to drizzle driver. Files requiring changes are: | ||
| * docker-compose.yml | ||
| * fineract-provider/build.gradle | ||
| * fineract-provider/src/main/resources/META-INF/spring/jdbc.properties | ||
| * kubernetes/fineract-server-deployment.yml | ||
|
|
There was a problem hiding this comment.
Nah, this will be super confusing... we cannot (should not) have the release be anything different than what we work with during development. Let's figure out a way to avoid this. My proposal is that we simply do not ship any JDBD driver in the Fineract distribution at all, ever.
Instead, why don't we just include a 2-3 lines trivial small fineract.sh script illustrating how to launch java ... with both fineract.jar and mariadb-java-client-2.7.0.jar on the CP? And document that in https://github.com/apache/fineract#instructions-to-build-the-jar-file as well (and also a word about having to add it to Tomcat when using the WAR), as part of this PR. And then we could entirely remove this section for "special instructions" for Releasing. WDYT?
As for actually getting the mariadb-java-client-2.7.0.jar, it must be available somewhere under .gradle/ or so? I'm not just where you can find it... or we could just wget (or curl - whatever) the MariaDB Connector/J?
| @@ -80,7 +79,8 @@ dependencies { | |||
|
|
|||
| 'com.github.spotbugs:spotbugs-annotations', | |||
| 'io.swagger.core.v3:swagger-annotations', | |||
| 'org.webjars:webjars-locator-core' | |||
| 'org.webjars:webjars-locator-core', | |||
| mysqlConnector | |||
There was a problem hiding this comment.
So this, unfortunately, as I understand it, is No Go. This would mean that we include a LGPL into the distro. Or that we would have to make special handstands to remove it again when we release to distribute, which I fear will be too confusing.
What we CAN do, as far as my understanding goes, is to have a test only scope dependency. So for unit tests, it's on the classpath, but it's never "packaged" and distributed. We currently only have one Spring Integration Test, but I would still do add it, with another scope than implementation, but some... test only something (whatever the right name is).
fineract-provider/build.gradle
Outdated
| @@ -29,6 +29,8 @@ repositories { | |||
| } | |||
|
|
|||
| project.ext.jerseyVersion = '1.19.4' | |||
| project.ext.mysqlConnector = 'org.mariadb.jdbc:mariadb-java-client' | |||
There was a problem hiding this comment.
how about ditching the mysql prefix from the name here, and just calling this (and the following)... maybe jdbcArtifact and jdbcVersion?
So for |
|
Thanks for the input. I’ll look into it as soon as I can.
…On Sat, Oct 3, 2020 at 10:13 PM Michael Vorburger ⛑️ < ***@***.***> wrote:
Not sure if that applies also for running unit tests on the console, would
need to test.
So for ./gradlew integrationTest to work with what I have proposed above,
where I suggest that we do NOT include any JDBC driver JAR into the WAR, we
would just have to add it (a JDBC driver) to the external Tomcat which we
start for running the ITs. This, hopefully, shouldn't actually be that hard
- note the extraClasspath and sharedClasspath properties
<https://github.com/bmuschko/gradle-cargo-plugin#extension-properties>,
which we should be able to use somewhere in our Cargo plugin configuration
<https://github.com/apache/fineract/blob/develop/fineract-provider/build.gradle#L682>
?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1366 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AK5B5DBRSS6VGZUNDV2YE3LSI6HXXANCNFSM4SB4SH4A>
.
|
|
BTW we'll probably also have to do something to add the JDBC driver to the
classpath for ./gradlew bootRun.
|
|
This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days. |
FINERACT-982