Liquibase precondition conventions

Development
1

It says on Conventions about liquibase change sets: “Always include a precondition and a short comment for each changeset”

The documentation does not describe why one should include such a precondition. Preconditions are useful/required in many cases but in OpenMRS they often seem to be used for making redundant checks. The point of using solutions like liquibase is that liquibase takes care of applying change sets in order defined by change set ids and also executing only those change sets that have not been applied on the particular database instance.

For example, these kind of preconditions are common in OpenMRS. The precondition below marks the change set as ran if the table already exists:

    <changeSet id="1234" author="foo">
        <preConditions onFail="MARK_RAN">
            <not>
                <tableExists tableName="mytable"/>
            </not>
        </preConditions>
        <comment>...</comment>
        <createTable tableName="mytable">
        </createTable>
    </changeSet>

Now how can the table exist if the change set has not been ran by liquibase? Has someone added it manually? Do we want to support that kind of administrative workflows? Another example:

    <changeSet id="4321" author="foo">
        <preConditions onFail="MARK_RAN">
            <columnExists tableName="mytable" columnName="mycol"/>
        </preConditions>
        <comment>Drop the not null mycol contraint</comment>
        <dropNotNullConstraint tableName="mytable" columnName="mycol" columnDataType="int"/>                
    </changeSet>

I don’t see the point of the precondition. It skips the change set and marks it as ran if the column “mycol” does not exist. This seems pointless as the database is probably in an unusable state if the “mycol” column is missing. I would rather use onFail=“HALT” or just not use a precondition at all. Both options would tell the admin: “Your database is in an unusable state because mycol is missing”

I guess these kind of preconditions may not break anything directly but they are in my opinion redundant, misleading and may advocate manual modification of database schemas.

What do you think?

2

I think that for a PROD environment onFail=“Halt” would be ok (DB scripts shouldn’t be reentrant).

But for a DEV environment the other option can be useful. It’s pretty common to work with different versions of the soft and the same DB (ideally each version should be run with its particular DB version). Sometimes the startup process fails, etc.

3

The rationale behind this (and it would be nice to document this on the wiki) is that we’d like to be able to recover from the scenarios where (a) you have a database, but the liquibase log tables are lost, or (b) someone is manually splicing together parts of databases, things get messed up, and their recovery path is to delete from the liquibase log tables and restart OpenMRS.

For example the currently-recommended way of installing updates to the CIEL concept dictionary calls for running a SQL script against your DB that overwrites the concept tables, but ignores the liquibase ones.

(So, you’re right, they do “advocate manual modification of db schemas”, but that’s intentional, although it’s non-ideal.)

It’s a fair question though…should we consider changing our practice here? Can anyone speak to having had to drop their liquibase log tables and restart OpenMRS, in recent years? (I remember seeing this on the lists quite frequently years ago, but hopefully we’re in a better place now!)

4

For older modules, one of the main reasons this is done is because they often started out with sqldiff as the mechanism for applying database schema changes, and then migrated over to using liquibase when this was introduced. In order for this transition to be made seamlessly, one adds preconditions with onFail=“MARK_RAN” in order to ensure consistency between installations that were done before and after liquibase migration.

Mike

1 Like
5

Also in some cases tech survey system admins wish to pre-configure things in the DB to meet their needs ahead of an upgrade so these preconditions allows them e.g to add columns and tables and pre populate them before they perform the upgrade.

6

I had forgotten about this point. It’s a very important one, and I wouldn’t want to give up this behavior.

7

Thanks for all the replies! As I suspected, things are newer as simple as a young developer would want them to be. I’m basically ok with using preconditions this way in these conditions. I took the liberty of writing a short rationale for preconditions at Conventions.

Still, a couple comments:

I have also used a dedicated database/schema for each branch I’m working on but I can see how this might be difficult if you need to maintain a specific set of data in each database, for example.

Did I understand you correctly, that admins are doing these modifications via manually crafted SQL? Wouldn’t it be possible in this case to just take a git checkout or a build package and run liquibase updates from command line (without actually deploying the package)?

8

I don’t think i understood your question @kosmik, as i said admins sometimes can have very specific data they may wish to get into their database prior to an upgrade and that’s entirely up to them

9

@wyclif, I was referring to what you wrote about “add columns and tables”. The scenario I was thinking about was that a new software version requires changes to the db schema and an admin applies these changes before deploying the new software version. But perhaps it was I who had a misunderstanding.