Make it possible to run migrations as a library

[actgardner]

Instead of using a separate binary with it's own config format, I'd like to be able to import this package as a library in a single deployment binary with my app.

This change makes it possible to bypass the cli library and bring your own method of configuring database connections, etc.

[andrewpillar]

Thanks for the PR, I feel that the main thing blocking the use of this as a library is not being able to pass through a preexisting database connection, as you pointed out.

Regarding this, I think we could get away with just changing the database.Open function to accept the name of the database to use, followed by a connection like so,

func Open(name string, conn *sql.DB) (DB, error)

this would of course meaning having to create some utility function for wrangling the connection data out of the config.Config struct for the command line, but I don't see why that couldn't exist in the cmd package.

Now, when it comes to importing this package to use in an application to perform migrations, I feel that it is mostly possible to do this (assuming the aforementioned changes are made to the database package), and that performing the migrations is a matter of calling DB.Perform, handling the necessary errors, then calling DB.Log to log the revision.

Granted, some of this logic could perhaps be tidied up. For example the call to Revision.GenHash could be done within DB.Perform. But I feel that if a developer wanted to do migrations as part of their application they can already do this with what is available, whilst still giving them control over how to report the progress of these migrations.

Let me know your thoughts.

[actgardner]

Regarding this, I think we could get away with just changing the database.Open function to accept the name of the database to use, followed by a connection like so, this would of course meaning having to create some utility function for wrangling the connection data out of the config.Config struct for the command line, but I don't see why that couldn't exist in the cmd package.

I mean we could extract the DSN logic somewhere else, I think it's just going to result in a switch case somewhere else in the codebase to handle mysql/postgres/sqlite connection strings? To my mind it makes sense for each database implementation to handle mapping the config object to the DSN.

But I feel that if a developer wanted to do migrations as part of their application they can already do this with what is available

The pieces are all there, in our case it would just mean copy-pasting the contents of the half the private perform method. This package is already opinionated about how migrations should work, making it a public part of the API makes implementing migrations very simple:

	if err := db.Init(); err != nil && err != database.ErrInitialized {
		return fmt.Errorf("failed to initialize database: %v", err)
	}

	revisions, err := revision.Oldest()
	if err != nil {
		return err
	}
	return migration.Perform(db, revisions, revision.Up, false)

We could actually even move db.Init into migration.Perform.

whilst still giving them control over how to report the progress of these migrations.

I think exposing a logger interface is sufficient - the lower level API is still accessible if people want full control but I think this would cover 80% of cases.

[andrewpillar]

To my mind it makes sense for each database implementation to handle mapping the config object to the DSN.

Personally if this is going to be a library then I don't think it should really care where the database connection comes from, just that it has it. Furthermore adding a new method FromConn could make the API harder to understand, if we've already called database.Open and received our implementation, why do we need to call FromConn? Unless the user is straight up instantiating the concrete value. So I'd be fine with moving the DSN logic some place else.

This package is already opinionated about how migrations should work, making it a public part of the API makes implementing migrations very simple:

Yes it makes things simple, however I think this could also be used as an opportunity to provide a more succinct API. For example, perhaps we could change the Perform method to accept the parameters instead,

Perform(d revision.Direction, force bool, revisions ...*revision.Revision) error

that way we don't need to introduce a new package, and the logic would look something like,

if err := db.Init(); err != nil {
    // handle error
}

revisions, err := revision.Oldest()

if err != nil {
    // handle error
}

return db.Perform(revision.Up, false, revisions...)

this would break what we currently have though.

Regarding logging, I personally don't think libraries should log on behalf of the user, and this is something that can be done at their own discretion when interacting with the API.