Add migration hooks to allow Go functions to be run#985
Conversation
These can be run before or after migrations. This should be seen as anticipatory. There isn't really much added logic, it mostly reorganizes what's there.
talanknight
left a comment
talanknight
left a comment
There was a problem hiding this comment.
This looks like it'll be very helpful for allowing go logic to modify db state in the current migration process. It is a little difficult to see how it'll look and how easy it'll be to maintain in practice without an example. I've added just a few comments to spur discussion.
| } | ||
| if err := b.driver.Run(ctx, bytes.NewReader(qp.ReadUp()), qp.Version()); err != nil { | ||
| upVer := qp.ReadUp() | ||
| if upVer.PreHook != nil { |
There was a problem hiding this comment.
Instead of Pre/Post hooks, what if we decoupled them completely from a statement version? What I am envisioning would be each version could be either an SQL statement that is passed to driver.Run(ctx, ...) or a goFunction that is passed to something like driver.ExecuteFn(ctx, migrateFunction) but not both.
The benefit of this approach, in my opinion, is that it allows the sql.Tx to not have to be exported since there may be a db in the future that we rely on dirty bits instead of transactions. It also makes the migration steps a little easier to reason about since you can just step through each migration version sequentially.
There was a problem hiding this comment.
So to make sure I'm understanding, you are suggesting that rather than pass statements, that we encapsulate the entirety of a version in a Go function? That way a "basic" update function would just execute the statements, but then rather than pre/post hooks we can just mix in logic as needed?
| `{{ .Name }}: { | ||
| Statements: []byte(` + "`\n{{ .Content }}\n`" + `), | ||
| }, | ||
| `)).New("MainPage").Parse(`// Code generated by "make migrations"; DO NOT EDIT. |
There was a problem hiding this comment.
Having this comment above the package makes it a package comment, but this is generated to a package where not every file is auto generated.
There was a problem hiding this comment.
Oh, sorry. I see now you moved it to its own package.
…ng the db has no issues with removing the storage bucket. This prevents the plugin from deleting rotated credentials while session recordings still exist in the bucket. (#985) Co-authored-by: Danielle Miu <29378233+DanielleMiu@users.noreply.github.com>
3 participants
These can be run before or after migrations. This should be seen as
anticipatory. There isn't really much added logic, it mostly reorganizes
what's there.