openmeter icon indicating copy to clipboard operation
openmeter copied to clipboard

Add versioned migrations

Open GAlexIHU opened this issue 1 year ago • 0 comments

Currently Proposed Solution:

  • We have to use atlas to generate the diffs based on the ent schema we declare
  • Atlas doesn't support multiple histories in a single database/schema (plain english: you can't change the lock table) so for migration running we have to use stg else (golang-migrate in this case)
  • Ent's Schema.Migrate is replaced by tool/migrate custom utility for running migrations as part of Application Startup.

Current Workflow

  1. Do your changes in internal/ent, then atlas migrate --env local diff <diffName> creates the new migration files
  2. You can apply your migrations with
migrate -database "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable&x-migrations-table=schema_om" -path ./services/common/migrations/om up

Or you can set postgres.autoMigrate=true to run migrations at startup

Other useful commands

# Lint migrations (atlas linting is quite good)
atlas migrate --env local lint
atlas migrate --env local-om lint # For Cloud only referencing the OM migrations

# Validate migrations directory
atlas migrate --env local validate

Improvements on Current Workflow

  • Using x-migrations-table in the connection string is a burden running manually. tools/migrate sets it consistently when invoking from code. A possible direction going forward could be to build on that custom tooling for both the local dev scripts as well as CI & Deploy automations (with all the drawbacks of having custom tooling for this)

Open Items for followups

  • As we can't use atlas for running the migrations we cant use it to validate things not conflicting either (not same as migrate validate that simply checks the checksum). We'll have to add a test applying the two migration trees to check they don't conflict
  • Same as above, we'll also need a custom test to validate that the current main version can be migrated with the version to be deployed
  • The atlas/checksum makes it more likely things will conflict if 2 PRs add migrations around the same time, but to ensure we need to add atlas migrate validate to CI

Notes for Reviwers

  • There's a sister PR to this one, you can check it here
  • postgres.autoMigrate has a default value of false, which is different from the current behavior. IMO having default=false makes more sense, but it might make sense to not introduce this change in defaults

GAlexIHU avatar Aug 06 '24 15:08 GAlexIHU