search

kitodo / kitodo-production

Kitodo.Production is a workflow management tool for mass digitization and is part of the Kitodo Digital Library Suite.
http://www.kitodo.org/software/kitodoproduction/
GNU General Public License v3.0
63 stars 63 forks source link

Fix some typos in migration scripts #6149

Closed stweil closed 2 months ago

henning-gerhardt commented 2 months ago

This will break every installation which is maintaining their database with FlyWay:

[ERROR] Failed to execute goal org.flywaydb:flyway-maven-plugin:8.4.4:validate (default-cli) on project kitodo-data-management: org.flywaydb.core.api.exception.FlywayValidateException: Validate failed: Migrations have failed validation
[ERROR] Migration description mismatch for migration version 2.19
[ERROR] -> Applied to database : Set properies to unique not null
[ERROR] -> Resolved locally    : Set properties to unique not null. Either revert the changes to the migration, or run repair to update the schema history.
[ERROR] Migration checksum mismatch for migration version 2.61
[ERROR] -> Applied to database : 1769151973
[ERROR] -> Resolved locally    : 1654358955. Either revert the changes to the migration, or run repair to update the schema history.
[ERROR] Migration checksum mismatch for migration version 2.68
[ERROR] -> Applied to database : 1266171681
[ERROR] -> Resolved locally    : -964385541. Either revert the changes to the migration, or run repair to update the schema history.
[ERROR] Migration checksum mismatch for migration version 2.71
[ERROR] -> Applied to database : -1061746512
[ERROR] -> Resolved locally    : 3664929. Either revert the changes to the migration, or run repair to update the schema history.
stweil commented 2 months ago

There were already renames and other changes made for the migration scripts in the past as far as I know. Therefore I thought that running repair is something which was accepted.

stweil commented 2 months ago

For our own production database the existing scripts also produce checksum mismatches. So the documentation for migrations should include this case.

stweil commented 2 months ago

Unrelated: I wonder why even new migration scripts are named V2_*.sql. Are they really still related to Kitodo.Production 2.x? Or what is the meaning of "V2"?

henning-gerhardt commented 2 months ago

Unrelated: I wonder why even new migration scripts are named V2_*.sql. Are they really still related to Kitodo.Production 2.x? Or what is the meaning of "V2"?

V2 has nothing to do with Kitodo.Production 2.

V1 is the base for all upcoming migrations while developing for Kitodo.Production 3.x was ongoing until now and their version begins with V2 prefix.

henning-gerhardt commented 2 months ago

There were already renames and other changes made for the migration scripts in the past as far as I know. Therefore I thought that running repair is something which was accepted.

Yes this was done in the past and I was not happy over this changes as I maintain a lot of databases which must all adjusted when the databases get updated to a new version usage. Now it is not only a typo correction inside the files now even a migration file get renamed. The typo's may be ugly but this should be discovered while the pull request is reviewed and not a lot of time later and even after the file is part of a release. In my opinion this typo's should stay and not get fixed.

solth commented 2 months ago

I agree with @henning-gerhardt that fixing these typos in comments (!) is not worth creating additional hassle and workloads for administrators, especially when those typos do not really pose any problems. I will therefor not merge the pull request and close it instead.

stweil commented 2 months ago

Is there additional hassle? I have to run flyway:repair anyway and see no additional workloads.

stweil commented 2 months ago

@solth, issue #6139 might require changes which are not in comments and which also require flyway:repair. Therefore I think that fixing typos in comments can be done without any additional hassle and workloads.

henning-gerhardt commented 2 months ago

@solth, issue https://github.com/kitodo/kitodo-production/issues/6139 might require changes which are not in comments and which also require flyway:repair. Therefore I think that fixing typos in comments can be done without any additional hassle and workloads.

The difference is: in this pr only "comments" are changed which did not influence the migration itself. In Issue #6139 may the migration itself need be changed (if this is really needed on MacOS) which would be not nice but more understandable.

Using mvn flyway:repair did only "fixing" the entries inside the flyway database table without running the migration script itself which may or not lead to different database schema's if the changed migration file did something total different in its latest version.