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

Adding a goose provider #379

Closed
mfridman opened this issue Jul 8, 2022 · 5 comments · Fixed by #635
Closed

Adding a goose provider #379

mfridman opened this issue Jul 8, 2022 · 5 comments · Fixed by #635

Comments

@mfridman
Copy link
Collaborator

mfridman commented Jul 8, 2022

Target date port Provider code into main branch: November 18, 2023

WIP Updates

October 7, 2023 - add a *goose.Provider with unimplemented methods. See #596

October 9, 2023 - add package lock which contains a SessionLocker interface with a postgres exclusive session-level advisory lock implementation. See #606

October 14, 2023 - add collecting sources and merging migrations (internal). This new implementation enables 2 highly requested features and fixes an existing limitation. See #615, #616 (refactor)

October 16, 2023 - collapse all provider-related logic into package provider, add migration logic and start adding integration tests (~85% coverage). Also, add parallel test for postgres to exercise the SessionLocker. #617

October 26, 2023 - add package database with a Store interface. #623

October 30, 2023 - bug fixes, resolve naming collisions. Settle on Store implementation. #624, #626

Initial description

For a long time, one of the most requested features was the ability to have multiple instances of goose within the same runtime. E.g., #114, #351, etc.

The problem with the current goose package is the global state. Which works just fine for a single instance when used to create a single binary.

This issue tracks the work that adds a Provider abstraction to the goose library. This also gives us a chance to improve package ergonomics so the goose library feels idiomatic to your average Go developer. Remember, goose is a 10-year-old project and we (read Gophers) have learned a lot about best practices for writing idiomatic Go code.


Provider

At a high level, the idea is to have a goose.NewProvider entry point.

// NewProvider returns a new goose Provider.
//
// The caller is responsible for matching the database dialect with the database/sql driver. For
// example, if the database dialect is "postgres", the database/sql driver could be
// github.com/lib/pq or github.com/jackc/pgx.
//
// fsys is the filesystem used to read the migration files. Most users will want to use
// os.DirFS("path/to/migrations") to read migrations from the local filesystem. However, it is
// possible to use a different filesystem, such as embed.FS.
//
// Functional options are used to configure the Provider. See [ProviderOption] for more information.
//
// Unless otherwise specified, all methods on Provider are safe for concurrent use.
//
// Experimental: This API is experimental and may change in the future.
func NewProvider(dialect Dialect, db *sql.DB, fsys fs.FS, opts ...ProviderOption) (*Provider, error)

The first 3 args are mandatory.

As a library, goose does not care about which driver the caller is using, only the SQL dialect. It is now the caller's responsibility to create a *sql.DB session, match it to a dialect and pass it down. The dialect is now a well-defined type, making it much harder to do the wrong thing.

See Doc comments for more details.

This design future-proofs out ability to extend functionality.


Improvements

  1. Less ad-hoc logging from within the library. The binary should support both text and json.

As a library, goose has too many log.Print statements. And we might want to keep that, but only if -verbose is enabled.

  1. Keep CLI (goose binary) concerns out of the goose library.

For example, goose.Run and goose.RunWithOptions are CLI concerns and do not belong as functions in the core library. These functions take the command name and the args, so it's impossible to know what they do without reading all of main.go. The library should not know anything about CLI commands.

  1. Adding context.Context support for all methods that run migrations
// Up migrations
func (p *Provider) Up(ctx context.Context) error {
func (p *Provider) UpByOne(ctx context.Context) error {
func (p *Provider) UpTo(ctx context.Context, version int64) error {

// Down migrations
func (p *Provider) Down(ctx context.Context) error {
func (p *Provider) DownTo(ctx context.Context, version int64) error {

// Helper migration methods
func (p *Provider) Redo(ctx context.Context) error {
func (p *Provider) Reset(ctx context.Context) error {
func (p *Provider) CurrentVersion(ctx context.Context) (int64, error) {
@oliverpool
Copy link

oliverpool commented Sep 19, 2023

Since I proposed a new function in #599, this might be relevant to this discussion.

In the initial post, you proposed func NewProvider(dialect Dialect, db *sql.DB, dir string, opt *Options) (*Provider, error) {.
In the current PR #507 it is func NewProvider(dbDialect Dialect, db *sql.DB, opt Options) (*Provider, error) {.

I would suggest func NewProvider(dbDialect Dialect, db *sql.DB, migrationFolder fs.FS, opts ...func(*option)) (*Provider, error) {:

  • fs.FS is very convenient and would allow to drop the dir parameter:
goose.NewProvider(dbDialect, db, os.DirFS("/path/to/migrations"))

// if the fs is provided and you want a subfolder, use fs.Sub
fsys, err := fs.Sub(fsys, "migration/sub_folder")
// todo: handle error
goose.NewProvider(dbDialect, db, fsys)
// simple usage
goose.NewProvider(dbDialect, db, os.DirFS("/path/to/migrations"))

// advanced usage
goose.NewProvider(dbDialect, db, os.DirFS("/path/to/migrations"), goose.WithTableName("my_migrations"), goose.ApplyOutOfOrder, goose.Verbose(true))

What do you think?

@mfridman
Copy link
Collaborator Author

mfridman commented Oct 5, 2023

Thanks for the suggestions @oliverpool.

I've gone back and forth on option raw struct vs struct with getters vs functional options and everything in between.

tl;dr I think functional options are the way to go mainly for consistency and future-proofing. Some notes I had:

  • familiarity (the /v3 package already uses functional options)
  • if we use a struct and later decide to extend some other part of the API in a backwards-compatible way, we'll do that with structs, and there will be inconsistency

Taking a fs.FS isn't a bad idea.

The original thinking was to have a dir string so that by default assumes local filesystem and eliminates the caller from having to think about the underlying impl. And more specific use cases would have a goose.WithFilesystem for those that know how to work with fs.FS

@mfridman mfridman removed the /v4 label Oct 5, 2023
@mfridman
Copy link
Collaborator Author

mfridman commented Oct 6, 2023

Alright, I kind of like the the 2 suggestions above @oliverpool 👍

Going to settle on this API, merge it and continue to implement it.

func NewProvider(
	dialect Dialect,
	db *sql.DB,
	fsys fs.FS,
	opts ...ProviderOption,
) (*Provider, error) {
 ... 
}

And I did like the original design because it had Setter which could be documented, another popular pattern. But it comes with limitations as well. Either way, I do like documenting the behaviour with the function instead of on the struct fields.

// WithTablename sets the name of the database table used to track history of applied migrations.
//
// If WithTablename is not called, the default value is "goose_db_version".
func WithTablename(name string) ProviderOption {
 ...
}

mfridman added a commit that referenced this issue Oct 7, 2023
# Conflicts:
#	go.mod
#	go.sum
@mfridman
Copy link
Collaborator Author

mfridman commented Oct 7, 2023

Stubbed out a *goose.Provider in #596 (merged)

Will start porting over some of the implementations from #507 (unmerged)

And would also like to introduce a Locker interface and glue that with the new provider, taking the best parts of #575 (unmerged) by @roblaszczak

@mfridman
Copy link
Collaborator Author

This functionality has been added in v3.16.0 and higher.

A bit more detail in this blog post:

https://pressly.github.io/goose/blog/2023/goose-provider/#database-locking

Thank you to everyone for the feedback.

@mfridman mfridman unpinned this issue Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants