openmeter
openmeter copied to clipboard
Add versioned migrations
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.Migrateis replaced bytool/migratecustom utility for running migrations as part of Application Startup.
Current Workflow
- Do your changes in
internal/ent, thenatlas migrate --env local diff <diffName>creates the new migration files - 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-tablein the connection string is a burden running manually.tools/migratesets 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 validatethat 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 validateto CI
Notes for Reviwers
- There's a sister PR to this one, you can check it here
postgres.autoMigratehas 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