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 package database with Store interface #623

Merged
merged 8 commits into from
Oct 27, 2023

Conversation

mfridman
Copy link
Collaborator

@mfridman mfridman commented Oct 27, 2023

This PR adds package database which contains a generic Store interface, backed by one of the supported dialects.

type Store interface {
	CreateVersionTable(ctx context.Context, db DBTxConn) error
	InsertOrDelete(ctx context.Context, db DBTxConn, direction bool, version int64) error
	GetMigration(ctx context.Context, db DBTxConn, version int64) (*GetMigrationResult, error)
	ListMigrations(ctx context.Context, db DBTxConn) ([]*ListMigrationsResult, error)
}

These are all the methods the goose provider uses for managing database migrations and versioning.

This also unlocks the ability for users to bring their own Store, resolving issues like #520 #530 and others.


Note, this could have been split out into database.NewPostgres(...) Store, and so on, but I'd like for goose.NewProvider to be simple by default and feel like it comes with batteries included. Normal users shouldn't need to know about the Store abstraction and can get started with a few lines of code:

db, err := sql.Open("sqlite", ":memory:")
if err != nil {
	return err
}
p, err := provider.NewProvider(database.DialectSQLite3, db, os.DirFS("path/to/migrations"))
if err != nil {
	return err
}

@mfridman mfridman merged commit 4ec43df into master Oct 27, 2023
9 checks passed
@mfridman mfridman mentioned this pull request Oct 27, 2023
@oliverpool
Copy link

Honestly as a user, I don't see much difference for simple usecases between the current master:

db, err := sql.Open("sqlite", ":memory:")
if err != nil {
	return err
}
p, err := provider.NewProvider(database.DialectSQLite3, db, os.DirFS("path/to/migrations"))
if err != nil {
	return err
}

And what I proposed in #608:

db, err := sql.Open("sqlite", ":memory:")
if err != nil {
	return err
}
p, err := provider.NewProvider(storage.Sqlite3(), db, os.DirFS("path/to/migrations"))
if err != nil {
	return err
}

However I think the state.Storage interface provides a lot of advantages for advanced usage:

  • using a custom state.Storage is very easy (just provide it as first argument), compared to having to use 2 arguments: database.DialectCustom and WithCustomStore
  • the provider API gets harder to misuse: if you provide something that does not implement the interface, the compiler will yell at you (whereas currently if you give "non-existing" as first parameter, the code compiles but fails to run)
  • table name customization is directly managed by the state.Storage implementations (where it actually belongs)
  • no need for dispatch/lookup code (like in database/dialect.go)

I can understand that you don't want to consider my proposal further. If this is the case, please close my PR #608.

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.

2 participants