Added a fastForward migration method

[voxpelli]

Moving this code to its own PR as there were no comments on it when linked in #279 and as new PR:s like #513 that relates to migrations are getting submitted.

For now we should probably keep the general discussion about how to move forward with migrations in #279 and only have comments related to the code of specific PR in here. We might need many PR:s like this one to solve #279 as every contributor needs their own, but thats okay I think.

Also: This PR only adds a knex.migrate.fastForward() and refactors the migration system to minimize duplicated code in that method – the CLI tool requested in #279 has not been added – the CLI should in my opinion wait until a knex.migrate.fastForward() has been accepted.

What does this PR add?

This PR adds a knex.migrate.fastForward() method which just like knex.migrate.latest() creates a new batch and includes all non-run migrations in it, but unlike knex.migrate.latest() it doesn't actually execute any of the migrations.

I myself am interested in pairing knex.migrate.fastForward() with my projects' installation scripts to ensure that no old migration scripts gets run on fresh new installations – see eg. the scripts of my one-page project and WebMention project.

[voxpelli]

Ping @bendrucker I guess

[bendrucker]

Will review as soon as I have a chance. I'm going to try to captain getting things a little more organized around here. Stay tuned.

[voxpelli]

@bendrucker Fixed the failing tests now (but Travis CI seems to have an unrelated problem with SQLite at the moment?).

[bendrucker]

Yeah I just disabled 0.11 in 342766c because of SQLite. Not sure what's up. I just kicked off a fresh run.

[bendrucker]

Indeed it was. Passing.

[voxpelli]

Rebased on top of master so that this PR stays up to date.

How to use this new method:

// Installation script to set up application schema

// First set up the tables using up to date definitions...
knex.schema.createTable('foo', function (table) {
  table.string('id').primary();
  table.string('bar').notNullable();
}).then(function () {
  // ...and then mark the existing unneeded migrations as done
  return knex.migrate.fastForward();
});

As you see – this will ensure that only future migrations that are relevant to the current installation will be run when the next migration is run and not irrelevant migrations that are related to changes that happened before the current installation was made.

Don't want to be a pain @bendrucker, but some feedback on how to move this forward would be nice 😃

[bendrucker]

See https://github.com/knex/arctic

Pretty much everything but the public API is changing. A big issue I'd like to address is having a better test suite with a strong split between unit and integration.

And I still need to decide what the API is going to look like. There's too much overlap between a to method and fastForward. I'm personally partial to an array-like method, knex.migrate.to([migration, index]). migration, per #513, would be a timestamp or full name of a migration. index, a la [].slice, would be a Number specifying a direction from the specified migration to move for the actual migration. So rather than ship migrate.before(ts) right away, you'd do migrate.to(ts, -1). And if !arguments.length, it goes straight to the last migration.

[voxpelli]

@bendrucker Regarding the API – the reason why to() and fastForward() isn't the same in this PR is since to() will actually execute each individual migration while fastForward() will just mark them as complete.

So in your example, if there were to be just a to() then it would have to be knex.migrate.to([migration, index, fastForward]) or something – and three optional parameters quite much – and to replicate what I did with knex.migrate.fastForward() in my above example one would then have to call knex.migrate.to(undefined, undefined, true) and I think a shortcut like knex.migrate.fastForward() would be preferable then, even if knex.migrate.to() were to support fast-forwarding as well.

---

In regards to https://github.com/knex/arctic – if we can decide on an API then I can try to make a PR for it to that project.

[bendrucker]

Right you are. There's still a ways to go on the internal methods around listing the files and changing the database. I'll let you know when there's enough progress.

[carera]

Hi
since migrate.to and migrate.before are not being implemented, what would be the best practice of testing validity of my migration?

[elhigu]

Hi, I started going through old pull requests and get them merged if possible or close otherwise if they are not going to be finished.

@voxpelli do you think this is still relevant and useful feature to have? If so, conflicts needs to be fixed and documentation of the new feature added to index.html.

ps. I would rather just close this pull request and forget about this feature, since it is really marginal case, where people needs it.

Only case I was thinking that this is needed is if you copy just schema without data from other DB to new installation (would cause migrations table data not to be moved). IMO one should just run migrations for creating new schema instead of copying schema from some database dump. Or if you copy schema from DB dump, you should copy also migrations table rows so that migration table will be uptodate with schema.

[voxpelli]

@elhigu I still believe this is relevant and I have continued down this route in https://github.com/voxpelli/node-knex-migrator-extension which is basically a hackish way to apply this PR at runtime to the knex migrator.

I would happily move away from that patching and update this PR to incorporate any experiences found in the deployment of that tool if that would help move this PR forward.

One question though: Is the plan still to move to https://github.com/knex/arctic and thus that any feature additions to the migrator should still be targeting that as the last feedback from @bendrucker stated or is the plan now to keep the current migrator for the near future?

And regarding this being a marginal case: I very much disagree about that.

I think that a schema definition for a fresh install is a distinctly different use case from migrations of older schema definitions to new ones.

It should always be possible to set up a project by itself, without running all the migrations, and many tools does this – such as eg. Drupal which always on fresh installs sets up the current state of the schema and only runs migrations when it needs to migrate older installs to newer ones. Enabling one to do that is what I'm aiming at with this PR and that's how I currently use it in the above mentioned projects.

tl:dr; This is a DRY way to mark a fresh install as already including all of the migrations that was available at the time of the installation thus enabling projects to do what eg. Drupal does and keep a clear definition of their schema for new installs and leave migrations to just be migrations

[elhigu]

@voxpelli that drupal example sounds like schemafile vs. migrations discussion, which to me has not much to do with this fastForward feature (it requires more complete support and syncing migration table data is just one part of it).

In my point of view this feature this is just an easy way to combine any incompatible schema file with migration data that might or might not be valid for it.

That being said, I'm not going to push this pull request down since there has been already collaborator support for this 👍 Just expressing my view on this matter.

So does this require any additional PR's to be merged? When the conflicts could be fixed and documentation added?

ps. I haven't heard about @bendrucker / arctic for a while

[elhigu]

Closing due to inactivity, if someone likes to implement / fix this, please open another PR an link to this. Or if this is finished / tested / documented, it can be reopened.