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

feat(experimental): add collect migrations logic #615

Merged
merged 3 commits into from
Oct 14, 2023
Merged

feat(experimental): add collect migrations logic #615

merged 3 commits into from
Oct 14, 2023

Conversation

mfridman
Copy link
Collaborator

@mfridman mfridman commented Oct 14, 2023

This PR adds the Source type that represents SQL and Gp migrations files found in the file system.

Source files are an intermediate representation that gets merged with registered Go migrations to return a Migration object. (this is an internal implementation detail, most users don't need to know this).

This implementation keeps parity with the existing code, with 3 notable exceptions:

  1. Exit early if there are Go migrations in the file system that have not been registered at the time of initializing the Provider. Previously, we used to gather all the migrations but failed ONLY if/when running the migration, which leads to partial errors at runtime. Note, that the existing implementation remains as-is, only the Provider code is different.

Specifically, this check:

goose/migration.go

Lines 103 to 105 in fe8fe97

if !m.Registered {
return fmt.Errorf("ERROR %v: failed to run Go migration: Go functions must be registered and built into a custom binary (see https://github.com/pressly/goose/tree/master/examples/go-migrations)", m.Source)
}

  1. This implementation allows us to expose a goose.WithGo and goose.WithGoNoTx options to register migrations directly into the provider instead of relying on the init() registration functions.

This is particularly useful for DI codebases that create a closure over the Go migration functions to pass down configs, other clients, etc.

  1. Offer the ability to exclude migrations by filename using goose.WithExcludes as an option. Feature request: baseline/ignore migrations #431

One thought I had as I was implementing this is it becomes the caller's responsibility to know how to work with fs.FS.

I do think it's way more flexible for the caller, e.g., using embed.FS or fs.Sub, and it becomes less for goose to maintain, i.e., having an explicit dirpath and a fs.FS.

@mfridman mfridman mentioned this pull request Oct 14, 2023
@mfridman mfridman merged commit 68853f9 into master Oct 14, 2023
8 checks passed
@mfridman mfridman deleted the gh-379-2 branch October 14, 2023 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant