task: Rename eventProperty to eventField [DHIS2-18549]

[muilpp]

As agreed on slack, I'm renaming eventProperty to eventField, to align with our current behavior in the API.

[ameenhere]

Since we have not released v42, can we include this change to the original migration script and remove this script?

Instead of first creating something else and then altering it?

[teleivo]

What you are proposing would cause issues on play dev as the we would change the checksum of a migration that ran already. We would need to fix that manually AFAIK.

[muilpp]

this is not released, but it's been in master for a few days now.
I guess some people will have pulled from it already, if I use the original migration script, won't it create a conflict in Flyway for these people?

[ameenhere]

It would create some conflicts yes. For devs who already have applied the old migration, they will have to either manually fix it or start from an older snapshot.
I doubt it would be an issue in play dev, as that always uses a db snapshot from a while ago. Possibly v41 db snapshot. We also have the flyway repair option in dhis config to make the fixes slightly easier. For master play dev, I think we do have flyway repair always set to true (I think).

Main intention for this suggestion is to avoid these developmental evolution "within a release" forced onto production migrations.
It is better we take some developer convenience hit, rather than having more steps in the migration process than needed. Imagine the table already having millions of entries after the original migration, and in this second migration, we do something extra in that large table which could have been combined in the original migration.

Either way, my feeling is to fix (and communicate to devs how to fix) the conflicts for this. What do you think? I could be convinced otherwise :D

[muilpp]

I get the concern about adding more steps to the migration, but isn’t it pretty much transparent to the implementations? They’d just update to v42, and Flyway would handle the updates automatically. Whether it’s all in one script or split into a few shouldn’t really matter to them.

I see your point of view and would agree if the second script was performing an operation on the data, such as adding a new column to an existing index or modifying the data in some significant way. However, in this case, it's simply renaming a column. Even if the table contains millions of entries, the execution time should stay the same.

Would you agree?

[ameenhere]

To clarify, I was not suggesting to add a "Alter table rename..." to the original migration script. Rather change the create table script to have the correct column names the first time.

Whether it’s all in one script or split into a few shouldn’t really matter to them.

It will matter to them if we do split migrations would cost more time right? It may not be significant for this case though. Since the renaming column is potentially a constant time operation. But that constant time is still an additional time. If the original query, when creating the table the first time, if it had the right column names, this renaming wouldn't have been necessary.

In this case though, assuming (and hoping) the additional time is very very small, let us go with this separate step 👍 😄

[teleivo]

What you are proposing would cause issues on play dev as the we would change the checksum of a migration that ran already. We would need to fix that manually AFAIK.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud