galovics commented on pull request #2127: URL: https://github.com/apache/fineract/pull/2127#issuecomment-1062663276
thanks @vidakovic for the review, let me address your points: * -Werror, the best would be to leave it there but EclipseLink simply generates bytecode that has warnings in it. There's a potential fix for this since the -Werror flag comes in when the compilation happens. We could separate the static weaving and the actual source code compilation into 2 different steps, like the regular compileJava (this would have -Werror enabled) and a staticWeaving step which has -Werror disabled. The thing is, in that case we have to play around the classpath configurations and generate the statically weaved code somewhere else than the regular build directory and there's a simple explanation for that. If I separate the static weaving and compilation into 2 different tasks and the static weaving overrides the existing build dir content, the Gradle up-to-date checks are going to be broken and every single time you try to restart the app from IntelliJ, it will recompile everything even though nothing changed. * `DatabaseSelectingPersistenceUnitPostProcessor` very valid question. I needed to implement this last minute since I had the same impression. The current database support for Fineract is MySQL (and MariaDB) and PostgreSQL. The thing is, [EclipseLink only support MySQL and PostgreSQL but not MariaDB](https://www.eclipse.org/eclipselink/documentation/2.7/jpa/extensions/persistenceproperties_ref.htm#target-database). When I tried it with PostgreSQL, EclipseLink correctly recognized it's a PostgreSQL database but when I tried with MariaDB, it simply collapsed because it didn't recognize it as a MySQL database. I guess there's a difference what the JDBC driver says about itself when it's the MySQL driver or the MariaDB driver. * `EntityScanningPersistenceUnitPostProcessor` That's needed otherwise the entity classes should be listed in the persistence.xml * Good point, let me take a look there. * Yeah, might look strange but the ID generation strategy is a bit different with EclipseLink than OpenJPA. If we try to get the primary key in the response to a request when we just persisted a new entity, first we gotta get the DB to generate the ID, hence the explicit flushing. This worked differently with OpenJPA. * Agreed. Caching is a whole separate story which I want to cover later on. Logging, I was thinking the same but when I turned it on, it literally got unreadable so I might not want to expose this to be configurable via env vars. It's rather for development only in my mind. Let me know your thoughts. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
