cmd: database migrations should not be run automatically but have a cmd instead

[aeneasr]

No description provided.

[aeneasr]

Resolved in #445

[meyerzinn]

I would actually ask that migrations happen automatically, because as-is it's exceptionally difficult to include hydra in an automated sequence like docker-compose because it requires this extra step.

I think that by passing a database URL to Hydra, you are implicitly authorizing Hydra to do with it what it sees fit. That means it's a blank canvas, ready to be shaped by hydra itself to work. That takes out the human error part, and further allows Hydra to keep its database steady--for example, if there is a small change in an index between migrations and a user doesn't migrate manually, that index remains unfixed regardless of Hydra version. However, if Hydra did SQL migrations automatically, that would not be an issue.

See https://github.com/drone/drone 's use of https://github.com/rubenv/sql-migrate for what I'm talking about.

[aeneasr]

I think that by passing a database URL to Hydra, you are implicitly authorizing Hydra to do with it what it sees fit.

That doesn't include accidental loss of data because a database migration didn't do what it should do. Also:

Dockerfile

FROM oryd/hydra

ENTRYPOINT /go/bin/hydra migrate sql $DATABASE_URL; /go/bin/hydra host

EXPOSE 4444

Hydra is not responsible for your infrastructure set up, all of the infrastructure tools allow automation of this kind.

[meyerzinn]

Well, to make Hydra production ready it should have integration tests, which would fail fast if data is accidentally lost. The point is basically to couple a Hydra version with a specific migration to allow for a linear progression over time. It makes it more predictable to have Hydra set up the database exactly how it needs.

Just my two cents, but it wouldn't take much to add it.

[meyerzinn]

Also, this isn't mutually exclusive with a separate migration tool, rather it's complementary. So if you want to upgrade Hydra you could use the manual tool to test it to make sure it didn't somehow get through integration testing with a database bug.

[aeneasr]

Well, to make Hydra production ready it should have integration tests, which would fail fast if data is accidentally lost.

Absolutely, this being done in parts already. The migrations are executed linearly already, and up- as well as down- commands are implemented. It is noteworthy that downwards migrations might cause loss of data as they might (not the case at the moment) remove columns or even tables.

In general, perfect migrations for all environments and databases in the wild are very hard. Sometimes, operators make small fixes to the schema in order to get better performance from the database. Until now, they had no possibility to run migrations manually. This is now the case.

In general, it is the responsibility of operations to properly execute software upgrades (e.g. with release canaries), and separating migration from the actual server is only benefitial here. In fact, I was approached by operations and they asked me specifically to disable auto-migrations and provide a separate command for it, with good reasons for it.

In any case, you are not loosing the ability to execute migrations and run the server in one thing, as I pointed out with the docker example above. You need to be aware of the implications though, which is also why those two commands are now separate.

[aeneasr]

By the way, the ladon migration (the only real thing that changed large portions of the schema) is fully tested - it still needs to be run manually in order to make 100% sure that you don't loose any data due to some weird quirk, as timeouts for example.