feat: dev flag for generating migrations

[ken-kost]

#490

I think the fix here is actually to support a --dev flag for migrations, which uses numbered migrations like Dev1 and Dev2 etc. Then, when migrations are checked with mix ash.codegen --check, we error if there are dev migrations. When you run mix ash.codegen name we rollback all dev migrations, delete all dev snapshots and regenerate them fully.

For now I'm just adding _dev everywhere and I left the timestamp which gives it uniqueness.
For now just deleting _dev files when --dev is not provided.

Not sure if in the right direction, need feedback. Putting this into draft.

Contributor checklist

• Bug fixes include regression tests
• Features include unit/acceptance tests

[ken-kost]

To try out make some changes and run
mix ash_postgres.generate_migrations --dev
it will create snapshots and migrations with _dev ending.
mix ash_postgres.generate_migrations
will remove all migrations and snapshots with _dev ending and continue as usual.

[zachdaniel]

This is great! I've got a busy week ahead of me, but will take a look when I get the chance 🙇

[zachdaniel]

So @sevenseacat has made two good points about the current solution here:

1. we need to rollback these migrations in the dev & test databases before deleting them. I think (will need some experimentation) that we should do that with System.cmd("mix", ["ecto.rollback", "--to", "<the earliest migration id>]), and then again with MIX_ENV=test in the environment. We will also need to roll it back for any tenants if there are tenant migrations. The logic for that is a bit hairier but basically consists of, for each env, listing all tenants and and running the same command with --prefix <the tenant>.

2. we should add a variation of this task that works using git history. What it would do is mark all uncommitted migration files as dev migrations, and then go through this same path. Something like mix ash.codegen --squash-uncommitted

I think #1 will need to be a part of this PR because otherwise we'll be putting users dbs in a bad state (they can't roll back the migrations because they've been deleted).

#2 can be done in a followup.

[ken-kost]

alright, thanks for direction. working on #1, will remove last commit which is refactor, that's for seperate PR. 👍

[zachdaniel]

Do the tests cover the rollback behavior?

[ken-kost]

Do the tests cover the rollback behavior?

right, I'll expand it calling migrate. 👍

[ken-kost]

@zachdaniel
Okay so I'm having some trouble doing the migration in test, my idea was to assert it was made successfully both times after generating migrations, dev and regular.
What happens is that twice the same extension migration is made (for sandbox repo and for test repo) which breaks the migration. For instance if I do
Mix.Tasks.AshPostgres.Migrate.run(["--migrations-path", "test_migration_path"])
I would get

** (Ecto.MigrationError) migrations can't be executed, migration name migrate_resources_extensions_1 is duplicated

The duplication happens because of the two repos that create_extension_migrations runs on, making two same named extension migrations.
Also haven't found any test running migrate or rollback.? 🤔
Do you have an idea how should I solve this? I'm thinking somehow getting rid of the sandbox repo maybe, I'm not sure. 🐛

[zachdaniel]

Hmm...I think we should be making the sandbox and non sandbox repo use a different path for their migrations then?

[ken-kost]

Hmm...I think we should be making the sandbox and non sandbox repo use a different path for their migrations then?

Okay so it gets tricky. Because migration path for any migration is determined:

  defp migration_path(opts, repo, tenant? \\ false) do
    # Copied from ecto's mix task, thanks Ecto ❤️
    config = repo.config()

    if tenant? do
      if path = opts.tenant_migration_path || config[:tenant_migrations_path] do
        path
      else
        priv =
          AshPostgres.Mix.Helpers.source_repo_priv(repo)

        Path.join(priv, "tenant_migrations")
      end
    else
      if path = opts.migration_path || config[:tenant_migrations_path] do
        path
      else
        priv =
          AshPostgres.Mix.Helpers.source_repo_priv(repo)

        Path.join(priv, "migrations")
      end
    end
  end

So it's either configured through opts or is AshPostgres.Mix.Helpers.source_repo_priv(repo). So if migration_path is set then both repo's hit that path. If not then it hits existing migrations, but we are creating here temporary migrations in a temporary folder right, also, using existing macros like defposts trigger in migration a CREATE TABLE posts which already exists in existing migrations (I did circumvent this by adding a macro that creates a new table but I still had issues) .

This simple assertion turned out way harder to prove than anticipated. 😓

[ken-kost]

I'll work on it more, try to find a path. 🐛