feat: database migrations #3457

stuartwdouglas · 2024-11-22T02:57:16Z

No description provided.

alecthomas · 2024-11-22T10:38:46Z

backend/protos/xyz/block/ftl/v1beta1/provisioner/resource.proto

@@ -41,6 +42,12 @@ message MysqlResource {
  MysqlResourceOutput output = 1;
 }

+message MigrationResource {


Please prefix all these types/functions with "SQL", "migrations" is not specific enough.

Just to clarify why I think this is important - it's very likely there will be other kinds of migrations in the future.

backend/provisioner/migration_provisioner.go

backend/provisioner/registry.go

alecthomas · 2024-11-22T10:55:33Z

go-runtime/goplugin/service.go

+	Schema          *schema.Schema
+	Dependencies    []string
+	BuildEnv        []string
+	AdditionalFiles []string


I'm 95% sure this should just be part of the ModuleConfig .proto rather than here (cc @matt2e) - this is exactly what it's designed for. Each language plugin then just returns the directory for any migrations, and the build engine does the bundling rather than each runtime having to have custom logic.

I wasn't sure if we wanted the build engine to modify the schema after the language plugin was finished with it. I can definitely change it but I think it does kinda blur the lines about what is responsible for generating the schema.

I figured language plugins could just re-use the ExtractMigrations function and achieve re-use that way.

It's not a requirement that language plugins be in Go, and in general we try to push as much as possible out of the plugins and into the engine for this reason.

The pattern is that the ModuleConfig tells the BuildEngine where to look for different parts of the build, and the BuildEngine does the work of collecting it into the schema+build. This fits that fairly well. It could be a glob, or a regex, or a map of db names to paths or something.

I have changed this.

internal/migration/extract_migration.go

internal/migration/extract_migration_test.go

alecthomas · 2024-11-22T11:06:09Z

internal/migration/extract_migration_test.go

+		targetDir := t.TempDir()
+
+		// Create dummy migration files
+		migrationSubDir := filepath.Join(migrationsDir, "testdb")


Didn't want to use testdata for the fixtures?

I thought that was just for integration tests? Also it's possible Chad may have written this test...

Hahaha 😂

Nope testdata is standard Go for any test that has fixtures.

internal/schema/parser.go

alecthomas · 2024-11-22T11:10:58Z

internal/schema/schema_test.go

@@ -729,6 +731,7 @@ module todo {
  secret secretValue String
  // A database
  database postgres testdb
+    +migration sha256:8cc04c75ab7967eb2ec82e11e886831e00b7cb00507e9a8ecf400bdc599eccfd


alecthomas · 2024-11-22T11:11:18Z

internal/schema/validate.go

+					switch md := md.(type) {
+					case *MetadataMigration:
+						if found {
+							merr = append(merr, fmt.Errorf("database %q has multiple migration metadata", n.Name))


stuartwdouglas · 2024-11-23T03:46:10Z

I think I have addressed everything, except for the questions around if this should be handled by the build engine or language plugins.

alecthomas · 2024-11-24T23:54:38Z

backend/controller/sql/testdata/go/database/database.go

-		return err
-	}
-	_, err = db.Get(ctx).Exec("INSERT INTO requests (id,data) VALUES ($1, $2);", req.Id, req.Data)
+	_, err := db.Get(ctx).Exec("INSERT INTO requests (id,data) VALUES ($1, $2);", req.Id, req.Data)


So much nicer!

alecthomas · 2024-11-24T23:55:33Z

backend/protos/xyz/block/ftl/v1/language/language.proto

@@ -34,6 +34,9 @@ message ModuleConfig {
  // LanguageConfig contains any metadata specific to a specific language.
  // These are stored in the ftl.toml file under the same key as the language (eg: "go", "java")
  google.protobuf.Struct language_config = 10;
+
+  // The directory containing the SQL migration files
+  string sql_migration_dir = 11;


What's the syntax of the directory? <sql_migration_dir>/<database_name/*.sql ?

No docs yet, but none of the DB stuff has docs yet.

This was referenced Nov 22, 2024

JVM Runtime Roadmap #2439

Open

Dashboard #728

Open

DX Roadmap #2436

Open

stuartwdouglas force-pushed the stuartwdouglas/migrations branch 11 times, most recently from d58d4a6 to 7aa111b Compare November 22, 2024 06:40

stuartwdouglas marked this pull request as ready for review November 22, 2024 07:32

stuartwdouglas requested review from a team and alecthomas as code owners November 22, 2024 07:32

stuartwdouglas requested review from wesbillman and removed request for a team November 22, 2024 07:32

stuartwdouglas force-pushed the stuartwdouglas/migrations branch from 42eafe8 to a3485a2 Compare November 22, 2024 07:51

stuartwdouglas requested a review from a team as a code owner November 22, 2024 07:51

stuartwdouglas requested review from worstell and removed request for a team November 22, 2024 07:51

stuartwdouglas force-pushed the stuartwdouglas/migrations branch 3 times, most recently from e88667f to d4f0282 Compare November 22, 2024 08:31

alecthomas reviewed Nov 22, 2024

View reviewed changes

stuartwdouglas force-pushed the stuartwdouglas/migrations branch 2 times, most recently from e90d83b to 59d6913 Compare November 23, 2024 03:06

stuartwdouglas force-pushed the stuartwdouglas/migrations branch 2 times, most recently from e03fdd8 to c0f8696 Compare November 23, 2024 03:37

feat: add database migrations

ebdb02e

stuartwdouglas force-pushed the stuartwdouglas/migrations branch from c0f8696 to 4a50e95 Compare November 24, 2024 23:27

feat: database migrations

0ea0da2

stuartwdouglas force-pushed the stuartwdouglas/migrations branch from 4a50e95 to 0ea0da2 Compare November 24, 2024 23:31

alecthomas approved these changes Nov 24, 2024

View reviewed changes

stuartwdouglas merged commit 2745c83 into main Nov 24, 2024
92 checks passed

stuartwdouglas deleted the stuartwdouglas/migrations branch November 25, 2024 00:00

stuartwdouglas mentioned this pull request Nov 25, 2024

SQL Migrations for JVM #3484

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: database migrations #3457

feat: database migrations #3457

stuartwdouglas commented Nov 22, 2024

alecthomas Nov 22, 2024

alecthomas Nov 22, 2024

alecthomas Nov 22, 2024

stuartwdouglas Nov 23, 2024

alecthomas Nov 23, 2024

stuartwdouglas Nov 24, 2024

alecthomas Nov 22, 2024

stuartwdouglas Nov 22, 2024

alecthomas Nov 24, 2024

alecthomas Nov 22, 2024

alecthomas Nov 22, 2024

stuartwdouglas commented Nov 23, 2024

alecthomas Nov 24, 2024

alecthomas Nov 24, 2024

stuartwdouglas Nov 24, 2024

feat: database migrations #3457

feat: database migrations #3457

Conversation

stuartwdouglas commented Nov 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartwdouglas commented Nov 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment