Add flyway-maven-plugin to plugins and update flyway related configuration #6159
Conversation
…ation - fix dependency - use main.basedir instead of basedir in path of flyway.properties and update url This allows running `mvn flyway:migrate` from the source root directory. Signed-off-by: Stefan Weil <[email protected]>
There was a problem hiding this comment.
I'm unsure about this pull request: Yes, it fixed some issues (old wrong dependency location, other basedir directory, needed new depedency for new flyway versions) but missed some parts (identical content in two files and one file is copied to the other on CI execution) and even made the flyway execution general available without the flyway profile.
With and without the profile and even without the configuration duplication: if you execute any flyway command then from the main directory the flyway command are executed for every defined module. So running a mvn flyway:info from the main directory would not display the information once no it is currently 15 times :-(
If you want it only once then you must change in the Kitodo-DataManagement module and run the command from there which then needs a local adjusted flyway.properties file for the flyway.locations configuration parameter (the path information here is depending on where the flyway command is executed - the line / option would look like without your changes).
I don't know which goals for which usage scenarios should be supported or not but I have the opinion that we had a lot of goals with a lot of usage scenarios on different places and no one knows which of them are supported and which not.
| flyway.url=jdbc:mysql:https://localhost/kitodo?useSSL=false | ||
| flyway.locations=filesystem:../Kitodo-DataManagement/src/main/resources/db/migration | ||
| flyway.url=jdbc:mysql:https://localhost/kitodo | ||
| flyway.locations=filesystem:Kitodo-DataManagement/src/main/resources/db/migration |
There was a problem hiding this comment.
If this changes come in than this file and flyway.properties.actions are identical and the copying while CI execution is not needed anymore which is resulting in deleting the copy line in the CI workflow file and the deleting of the flyway.properties.action file.
| <plugin> | ||
| <groupId>org.flywaydb</groupId> | ||
| <artifactId>flyway-maven-plugin</artifactId> | ||
| <version>${flyway-maven-plugin.version}</version> | ||
| <configuration> | ||
| <configFiles> | ||
| <!-- local configuration file --> | ||
| <!--<configFile>${main.basedir}/config-local/flyway.properties</configFile>--> | ||
| <configFile> | ||
| ${main.basedir}/Kitodo-DataManagement/src/main/resources/db/config/flyway.properties | ||
| </configFile> | ||
| </configFiles> | ||
| </configuration> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>com.mysql</groupId> | ||
| <artifactId>mysql-connector-j</artifactId> | ||
| <version>${mysql.version}</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.flywaydb</groupId> | ||
| <artifactId>flyway-mysql</artifactId> | ||
| <version>${flyway-maven-plugin.version}</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.mariadb.jdbc</groupId> | ||
| <artifactId>mariadb-java-client</artifactId> | ||
| <version>${mariadb-java-client.version}</version> | ||
| </dependency> | ||
| </dependencies> | ||
| </plugin> |
There was a problem hiding this comment.
You are coping the whole existing flyway profile and made the flyway execution available without the flyway profile. I don't know is this correct and wanted. In my opinion the flyway execution should only be possible if you add the flyway profile to the maven command and not without this profile.
Only one change is needed in the flyway profile: the adding of the new dependency org.flyway:flyway-mysql. Both other dependencies are already defined in the profile dependency.
| <groupId>mysql</groupId> | ||
| <artifactId>mysql-connector-java</artifactId> | ||
| <groupId>com.mysql</groupId> | ||
| <artifactId>mysql-connector-j</artifactId> |
There was a problem hiding this comment.
@henning-gerhardt, @solth, how should we proceed with this pull request? Would you prefer a separate PR for mysql-connector-j?
There was a problem hiding this comment.
Maybe a solution like in #6230 for the 3.7.x release branch as the change is needed for more recent MySQL versions too.
This allows running
mvn flyway:migratefrom the source root directory.