Add Support for Db based migration state persistence.

[sloops77]

A Store persists of file name, direction & time - each execution is persisted

[sloops77]

Inspired from & supersedes #58

[sloops77]

Example of a dbStore

/**
 * Created by arolave on 26/07/2016.
 */
var MongoClient = require('mongodb');

module.exports = MongoStore;

function MongoStore(dbUrl, dbTable) {
    this.mongoUrl = dbUrl || process.env.MONGO_URL || 'mongodb://localhost:27017/admin';
    this.collectionName = dbTable || 'migrations';
}

MongoStore.prototype.getCollection = function (callback) {
    if (this.db) {
        return callback(null, this.db.collection(this.collectionName));
    }
    var self = this;
    MongoClient.connect(self.mongoUrl, {}, function (err, db) {
        if (err)
            return callback(err);

        self.db = db;
        callback(null, db.collection(self.collectionName));
    });

}

MongoStore.prototype.load = function (callback) {
    return this.getCollection(function (err, c) {
        if (err)
            callback(err);

        c.find().sort({timestamp: 1}).toArray(callback);
    });
};

MongoStore.prototype.save = function (migration, callback) {
    return this.getCollection(function (err, c) {
        if (err)
            callback(err);

        c.insertOne(migration, {forceServerObjectId: true}, callback);
    });
};

MongoStore.prototype.reset = function(callback) {
    return this.getCollection(function (err, c) {
        if (err)
            callback(err);

        c.drop(callback);
    });
}

[LinusU]

Very cool!

What do you think about making each adapter (except file-store) a separate module? That way we don't have to ship with a mongo db dependency which will be installed for people using the file store.

Passing in --store-adapter mongo could lead to require('migrate-store-' + 'mongo')

Also, I'm not sure about table and db-url. I think that each store should decide on which options it want to accept.

I also think that the configuration of store should be done programatically somehow, so that the user doesn't have to repeat the same options every time they run migrations.

Any thoughts?

[sloops77]

1. Yeah - i didnt check in the mongo adapter for that very reason. wanted to get some feedback before finalising everything. This PR just sets eveything up.
2. sounds great
3. makes sense
4. What did you have in mind? A config file? something else?

[sloops77]

All right - did a changes to meet 1,2,3.
Split out filestore from node-migrateas well - for consistency. Package.json will need updating when we are happy to go forward.
Filestore and mongostore are currently in under my profile:
https://github.com/sloops77/node-migrate-mongostore
https://github.com/sloops77/node-migrate-filestore

Major changes for testing & the cli. Check it out and give me some feedback

[chasdevs]

Hey, this looks interesting; any updates on the status of this PR? I'd be interested in adding a mysql adaptor to this. db-migrate is a little bloated and unclean for my purposes.

[sloops77]

I havent heard anything for linus. Please add a mysql adapter and list the link on this pr.

[chasdevs]

@sloops77 - Here's my first crack at a mysql adapter: https://github.com/node-migrate/node-migrate-mysqlstore

I generally followed the styling of your node-migrate-mongostore

[sloops77]

Hey, im on hols till thursday. Ill take a look then in all probability. Cheers for the help!

[LinusU]

Sorry for the delay on this, I've been mening to take a look and this and also implement a postgres store for a while now...

Lets do this! I'll create a organisation for keeping all the stores in, how does that sound? I'll invite both of you straight away. I'm thinking we keep the main app under tj/node-migrate for now since it was him who started the project.

Also, how do you feel about using standard style as the code style? It's so easy to enforce and it removes discussions about the style...

[joaosa]

@LinusU Using standard sounds like a good idea :)

[chasdevs]

Cool, I moved the mysql store to the organization and updated with standardjs styling (locally; see below).

edit - Looks like I lost permission to push to the repo when changing orgs :/. I think you can restore those permissions since you are the owner @LinusU; mind doing that when you get a chance?

[LinusU]

Should be fixed now, all members should be able to edit all repos now

[LinusU]

I've actually been thinking a lot on how to make the very best migration framework, and finally took the time to clean up and commit some of my notes. This is not much at all, and I'm not saying that it's the way to do it, but I think that you might get some cool ideas from it.

This have been my thoughts on what should be node-migrate 2.0, and I hope that I can commit the readme soon and not just engine documentation. Again, nothing is set in stone, these are just my personal thoughts on how it could be.

https://github.com/LinusU/node-migrate/blob/next-gen-migrate/engine.md

[joaosa]

@LinusU I went through your text and it looks like a good structure :). I'd suggest adding a verification step a la sqitch, so one can ensure the migration did what was intended.

[LinusU]

Added documentation for core, cli, and migrations: https://github.com/LinusU/node-migrate/tree/next-gen-migrate

@joaosa that's a cool idea, I've never used it myself but that could probably be useful. I think it doesn't even need any extra implementation in the engine, we can just check if there is a .verify step and run that after .up.

[sloops77]

Feels very rdbms oriented and not so applicable for nosql stores like mongo
or redis. Primarily the issue i have is with the engine modify, view and
formatQuery functions. Ill have more time to feedback on thursday.

Cheers
Andres

On Tue, 11 Oct 2016 at 10:14, Linus Unnebäck notifications@github.com
wrote:

Added documentation for core, cli, and migrations:
https://github.com/LinusU/node-migrate/tree/next-gen-migrate

@joaosa https://github.com/joaosa that's a cool idea, I've never used
it myself but that could probably be useful. I think it doesn't even need
any extra implementation in the engine, we can just check if there is a
.verify step and run that after .up.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#67 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEkBE7oSNrj3jxMRroopTTzNSqXSvhbnks5qyzdegaJpZM4JX8GM
.

[LinusU]

Feels very rdbms oriented

Yes, that might be true, I'm 100% committed to keeping migrate an "Abstract migration framework for node". Let's not make it rdbms specific!

[sloops77]

I think the engine should just expose a "connection" via a getConnection function that is passed to the migrations up/down functions

[chasdevs]

Looks great!

Another option is just not exposing the view/modify functions. The core philosophy of this module seems to be that it only tracks state, and has zero opinions about how the user changes state inside of migrations. Seems like view/modify are just optional features that would save the user from having to import/configure mysql/postgres/mongo inside of each feature since they are most-likely tracking the migrations in the same store they are modifying. Maybe don't make it a core feature, but let some engines expose utility methods like that which users could take advantage of optionally (i.e. the mongo/mysql engines could expose a getConnection method per @sloops77's suggestion)?

For dry-run, you could just print the migration titles that would be run; that way it's just exposing the current state of the system and doesn't make any assumptions about the user's implementation (i.e. that there even IS a query to format).

[joaosa]

@LinusU @sloops77 Indeed sqitch is oriented towards RDBMSs and it wouldn't be aligned with an abstract migration framework. Following its structure/conventions wasn't my point. My suggestion was more in the lines of burrowing one idea from it, i.e.:

"The idea is simply to test that a deploy script did what it was supposed to do. Such a test should make no assumptions about data or state other than that affected by the deploy script, so that it can be run against a production database without doing any damage. If it finds that the deploy script failed, it should die." (source)

I find that often in live stage/production systems, the expectation that all migrations ran in succession and that state wasn't tampered with can sometimes be no more than an assumption. It is useful to be able to check if a particular DB change was indeed applied. E.g.:

node-migrations verify <applied-migration-key>
ok
...
node-migrations verify <not-or-no-longer-applied-migration-key>
not ok

Either way, feel free to take my suggestion or focus on other more pragmatic design aspects :)

[wesleytodd]

The original feature added here should now be implemented in #77. I know this has been pending for a long time, but @sloops77, if you could take a look at the readme to see if this satisfies your needs that would be awesome. The implementation is basically taken from this PR, just updated for all the other changes I made in the fork I had.

If you are satisfied, feel free to close this.

[wesleytodd]

Closing from inactivity