getodk / central-backend

Node.js based backend for ODK Central
https://docs.getodk.org/central-intro/
Apache License 2.0
50 stars 76 forks source link

Move off of using Knex for migrations #744

Open matthew-white opened 1 year ago

matthew-white commented 1 year ago

We currently use Knex for migrations. At one point, we used Knex for all things related to the database. However, in v1.2, we moved to Slonik for everything except migrations. Mostly it's been fine to continue to use Knex for migrations, but it's required us to toggle between two pretty different SQL libraries. We're also on an old version of Knex, and we know that there are breaking changes on the way to the latest version.

Slonik doesn't have its own framework for migrations, at least when I last checked. However, @alxndrsn made the point that we could probably replicate the important parts of Knex's migration framework ourselves without too much difficulty. We could then rewrite previous migrations, converting from Knex to Slonik or postgres.js.

However, one challenge is that we don't have tests of most migrations, so it would be hard to verify these changes. We also still have high-level questions about how to approach migration tests (#466).

One idea that we've had is to take a snapshot of the schema of a particular past version of Central (version X), then remove all migrations from before that version. Users on a version of Central older than version X would need to upgrade to version X before upgrading to the latest version. If we took this approach, we wouldn't need to rewrite/convert migrations older than version X or write tests for them.

To facilitate this strategy, we're going to try to use as little Knex as possible going forward. That is, we should write as much raw SQL as possible: for example, db.raw('INSERT ...'), not db.insert(...). Writing raw SQL is what we usually do outside migrations, so using more raw SQL in migrations should make it easier when we start rewriting migrations that are newer than version X.

We should also try to test new migrations going forward and continue thinking about #466.

alxndrsn commented 1 year ago

With a new approach to database migrations, is a down implementation still wanted for reversing individual migrations?