Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run migrations programmatically #167

Closed
IlyaSemenov opened this issue Aug 29, 2023 · 6 comments
Closed

Run migrations programmatically #167

IlyaSemenov opened this issue Aug 29, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@IlyaSemenov
Copy link
Contributor

IlyaSemenov commented Aug 29, 2023

I am trying to use rake migrations inside a Nuxt 3 project, which is a bit unusual setup. Migrations must be loaded with dynamic import rather than traversing the directory (so that they get bundled with nuxt build), but there is no import.meta.glob (nuxt/nuxt#22106), and the dynamic import is eager, meaning that the actual module is pre-loaded on app start, not when import is called:

// This code alone triggers imports, even if the actual functions are never called:
const migrations = {
  "20230804101735_create_image": () => import("./migrations/20230804101735_create_image"),
}
console.log(migrations)

Since all migrations call change on the module level, that means all of them are executed when simply preparing the migrations collection:

import { change } from '../dbScript';

// Due to eager import, this gets called for every migration
change(async (db) => { ... })

Additionally, there is no CLI subcommands support yet (nuxt/cli#62) and reusing Nuxt infrastructure from a third party runner such as vite-node is complicated, so I went with running migrations from an API handler instead of a CLI script. Migrations require a statically exported change which is created by a single call to rakeDb on the module level. However, in this scenario the rakeDb args are not yet known on the module level: they will be only known later in runtime (as passed to API handler by the client).

I actually managed to achieve what I wanted with some hacks:

import { rake } from "../db/rake"

export default defineEventHandler(async (event) => {
  // TODO: check access
  const body = await readRawBody(event)
  assert(body, "Body is empty.")
  await rake(body.split(/\s+/))
  // TODO: somehow gather and return migration results
  return { ok: true }
})

the main rake file:

import { rakeDb } from "rake-db"

import { config } from "../config"
import { BaseTable } from "./base"

const migrations = {
  "20230804101735_create_image": () => import("./migrations/20230804101735_create_image"),
}

const createChangeFn = () => rakeDb({}, { baseTable: BaseTable })

export type Change = ReturnType<typeof createChangeFn>

export function defineMigration(callback: (change: Change) => void) {
  return callback
}

export async function rake(args: string[]) {
  const change = rakeDb(
    { databaseURL: config.database_url },
    {
      baseTable: BaseTable,
      migrationsPath: "./migrations",
      migrationsTable: "rakedb_migration",
      migrations: Object.fromEntries(
        Object.entries(migrations).map(([name, load]) => [
          name,
          async () => {
            const module = await load()
            module.default(change)
          },
        ]),
      ),
    },
    args,
  )
  await change.promise
}

and the migration:

import { defineMigration } from "../rake"

// This works even with eager imports
export default defineMigration((change) => {
  change(async (db) => {
    await db.createTable("image", (t) => ({
      id: t.integer().primaryKey(),
      ...t.timestamps(),
    }))
  })
})

I propose to rework the migrations architecture to allow first class support of what I've done above, in particular:

  • separate the CLI runner and collector of change callbacks
  • allow to provide change callbacks in runtime rather than on module level

I suppose this could be an extension (retaining backward compatibility) but I have not considered the specific APIs yet. Let me know if this makes sense at all?

@IlyaSemenov
Copy link
Contributor Author

IlyaSemenov commented Aug 30, 2023

The new API could look like:

import { createRakeDb } from "rake-db"

export const { rakeDb, defineMigration } = createRakeDb(
  { databaseURL: config.database_url },
  {
    baseTable: BaseTable,
  },
  // all same arguments as in { rakeDb } from "rake-db", but no string args
)

// importing { rakeDb } from "rake-db" is still possible! This is only an extension.

then somewhere in a CLI script or in some other event handler:

import { rakeDb } from "../my-rake-db"

const myhandler = () => {
  await rakeDb(args)
  // or possibly:
  await rakeDb(args).promise // use the same return type as { rakeDb } from "rake-db"
}

in migration:

import { defineMigration } from "../my-rake-db"

export default defineMigration((change) => {
  change(async (db) => {
    await db.createTable("image", (t) => ({
      id: t.integer().primaryKey(),
      ...t.timestamps(),
    }))
  })
})

The migration runner could simply check that if a migration module has a default export and that's a function, call it as module.default(change).

That will be fully backwards-compatible with the current API, but give more flexibility.

@IlyaSemenov IlyaSemenov changed the title Eager import compatible migrations Run migrations programmatically Aug 31, 2023
@IlyaSemenov
Copy link
Contributor Author

Also, I would like this: await change.promise to return a boolean (whether the command succeed or not).

@romeerez
Copy link
Owner

romeerez commented Sep 1, 2023

dynamic import is eager

Lol wut? Dynamic is eager. It's impossible to stop surprising in our evolving ecosystem. Honestly, it's the weirdest bug I met in a long time.

I'm proposing to let migrations have a default export while keeping all the rest of it unchanged:

import { change } from '../dbScript';

export default change((db) => {
  // ...
})

Sometimes we need multiple changes per file, in such a case let's export array of changes:

import { change } from '../dbScript';

export default [
  change((db, up) => {
    // ...
  }),
  
  change((db, up) => {
    // ...
  }),
]

The internal logic will check if there is a default use it, if no default get callbacks from where change is putting them.

For the rakeDb I propose the following:

import { rakeDb } from '../src';

export const { change, run } = rakeDb.lazy({ ... })

To add a lazy constructor, the name suggests it won't perform anything until asked.

change is the same function as it is now - to import and use in migrations.

run is accepting an array of cli args as the rake does in your example. run will return a promise.

How does it look to you, would it be enough?

await change.promise to return a boolean (whether the command succeed or not).

Promise is expected to throw (reject) on error, I'll check if that is currently so.

@IlyaSemenov
Copy link
Contributor Author

dynamic import is eager. Honestly, it's the weirdest bug I met in a long time.

It's somehow related to preparing the package to run on edge infrastructure where there are no dynamic imports. They inline imports which gives that side effect in the dev version. They argue that one should never rely on module import side effects and I tend to agree with them in that.

How does it look to you, would it be enough?

Yeah, seems good!

Promise is expected to throw (reject) on error, I'll check if that is currently so.

Right, I actually concluded so later, too — please ignore that.

@romeerez
Copy link
Owner

romeerez commented Sep 3, 2023

Ready!

rakeDb.lazy docs

default export in migrations

Added forceDefaultExports boolean option to rakeDb config.

Hope it will help with your setup.

@romeerez romeerez closed this as completed Sep 3, 2023
@romeerez romeerez added the enhancement New feature or request label Sep 4, 2023
@IlyaSemenov
Copy link
Contributor Author

I just happened to migrate to this new API. Works great! Thanks so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants