WIP : Implement more migration features, fix a rollback all bug #3169

leeallen337 · 2019-04-28T16:25:26Z

This pull request aims to address a few issues (e.g. feature request and a bug) that I will elaborate on below.

rollback all running in wrong order #3131 - This bug looks like when trying to rollback all migrations it's running them in chronological order and, I think, attempting to rollback all migrations and not just the ones completed. From this line in the Migrator, val[0] I think is a list of all migrations in chronological order rather than only attempting to rollback the completed migrations in reverse chronological order.
See Bugfix/rollback all wrong order #3172 for a pull request which address this specific bugfix
Migrate to version #352, Migrate to the desired migration. #1550 and It would be great to be able to run an individual migration up/down #666 - These issues are requests to support migrating or down to specific versions and just migrating up one migration at a time or rolling back one migration at a time. I've implemented functionality to run migrate:up or migrate:down, which will migrate only the next migration or rollback only the last migration, respectively. There is also an optional --to flag which you can specify a specific the filename of the migration to migrate to (inclusive) or rollback to (exclusive) which I think addresses the issue.
~~Feature request: migrate:reset #783 - I also add in the --all flag to the migrate:rollback cli command which I think would address this issue~~
See pull request Feature/add rollback all to cli #3187
Knex migrations refresh #2907 - This issue is kind of addressed. There's not a command to rollback everything and run latest in one command but a combination of migrate:rollback --all and migrate:latest would address this issue
See pull request Feature/add rollback all to cli #3187

I've marked this issue as still a work in progress for a few reasons.

I wanted to get some feedback/comments/suggestions etc. from the maintainers about the functionality and implementation in this pull request before proceeding any further.
There are probably some things missing like tests, documentation, etc. I see several test files, is there a specific file tests for these should live in?

Possible ToDos:

Add tests
Update documentation

… --to flag to specify a specific migration

…n --to flag to specify a migration (exclusive)

…splice

kibertoad · 2019-04-28T18:30:08Z

@leeallen337 Could you please split this in multiple PRs, each of which would address just one thing?
Speaking of CLI tests - check out https://github.com/tgriesser/knex/tree/master/test/jake

kibertoad · 2019-04-28T18:31:10Z

bin/cli.js

@@ -202,13 +202,39 @@ function invoke(env) {
        .catch(exit);
    });

+  commander
+    .command('migrate:up')
+    .description('        Run the next migration.')


Description seems inaccurate.

What are your thoughts on Run the next chronological migration that has not yet been run.?

kibertoad · 2019-04-28T18:31:15Z

bin/cli.js

@@ -222,6 +248,29 @@ function invoke(env) {
        .catch(exit);
    });

+  commander
+    .command('migrate:down')
+    .description('        Rollback the last migration performed.')


Description seems inaccurate.

What are your thoughts on Undo the last previous migration performed.?

I'm more concerned about
a) usage of --to flag making description completely incorrect;
b) difference between last migration and last migration batch being not emphasized enough;

Oh I see what you mean. That makes complete sense! When working on a pull request for this I'll try and think of a more accurate and all encompassing description 👍

kibertoad · 2019-04-28T18:34:44Z

Bugfix for rollback order is very much welcome and would probably warrant one more release in 0.16.x line just for that change alone.

--up and --down commands make sense to me.

Could you please explain how differently --all is supposed to work from just a simple rollback?

kibertoad · 2019-04-28T18:35:02Z

Oh, and documentation should be updated here: https://github.com/knex/documentation

leeallen337 · 2019-04-28T18:45:42Z

@kibertoad Happy to split these up. I'll make some new smaller individual pull requests to break these up. I'll tackle the bugfix rollback order one first since that's a bugfix rather than a new feature.

Could you please explain how differently --all is supposed to work from just a simple rollback?

If you had batch 1, batch 2, and batch 3, you would have to run migrate:rollback three separate times to get to your initial state since, on its own, it just rolls back the latest batch. In the same scenario, you could run migrate:rollback --all just once and you would achieve the same thing which would rollback all batches. Hope that makes sense

kibertoad · 2019-04-28T18:48:19Z

Appreciate it!
Thank you for the explanation, yeah, I think that feature indeed makes sense.

leeallen337 · 2019-04-28T21:00:00Z

@kibertoad I created a separate pull request for the rollback all bug. The pull request can be found at #3172

I'm thinking of leaving this pull request open to serve as kind of a table of contents and documentation for these smaller ones I'm making. I can close this one after all these smaller ones have been finished. How does that sound? I also don't want to clog up the list of pull requests either so I could just go ahead and close this now too. Let me know what you think

kibertoad · 2019-04-28T21:59:13Z

@leeallen337 Sure, no problem! Appreciate the contribution :)

kibertoad · 2019-05-19T20:29:32Z

Is anything still left?

leeallen337 · 2019-05-20T12:57:58Z

@kibertoad Just a couple more things. The migrate:down functionality and then adding a --to <specifiy migration> flag to migrate:up and migrate:down so you can run specific migrations in a batch or roll back a group of specified migrations

andrewvsk · 2019-07-16T11:37:41Z

src/migrate/Migrator.js

+  }
+
+  const indexOfToMigration = findIndex(migrationCopy, (migration) => {
+    return migration.file === toFile;


Assume we have following migrations:

20001101000001_initial.js

20001102000002_migration1.js

20001103000003_migration2.js

Here is we have three possible migrate 'up/down to' cases:

to filename - 20001102000002_migration1.js

to timestamp (equal to) - 20001102000002

to timestamp (lower / upper bound)

up to 20001103000000 = migrationsToRun: [20001101000001_initial.js, 20001102000002_migration1.js]

down to 20001102000000 = migrationsToRun: [20001103000003_migration2.js, 20001102000002_migration1.js]

use short form 20001103, or pattern 2000??03, or regexp: 20001103.*.js

Actually you are running migration to exactly specific file name.
What do you think about other cases ?

leeallen337 added 7 commits April 25, 2019 22:31

Add :up migration command to migrate only one migration with optional…

8eef3ef

… --to flag to specify a specific migration

Add :down migration command to rollback only one migration with optio…

4af9409

…n --to flag to specify a migration (exclusive)

Add flag to allow rollback of all migrations

c971170

Swap lodash filter out for Array#filter

2ebc1d2

Refactor common functionality into one function

1785e45

Add types

f22446a

Use spread operator to create shallow copy of array instead of Array#…

177db4c

…splice

kibertoad reviewed Apr 28, 2019

View reviewed changes

andrewvsk reviewed Jul 16, 2019

View reviewed changes

kibertoad closed this Dec 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP : Implement more migration features, fix a rollback all bug #3169

WIP : Implement more migration features, fix a rollback all bug #3169

leeallen337 commented Apr 28, 2019 •

edited

Loading

kibertoad commented Apr 28, 2019

kibertoad Apr 28, 2019

leeallen337 Apr 28, 2019

kibertoad Apr 28, 2019

leeallen337 Apr 28, 2019

kibertoad Apr 28, 2019

leeallen337 Apr 28, 2019

kibertoad commented Apr 28, 2019

kibertoad commented Apr 28, 2019

leeallen337 commented Apr 28, 2019

kibertoad commented Apr 28, 2019

leeallen337 commented Apr 28, 2019

kibertoad commented Apr 28, 2019

kibertoad commented May 19, 2019

leeallen337 commented May 20, 2019

andrewvsk Jul 16, 2019

WIP : Implement more migration features, fix a rollback all bug #3169

WIP : Implement more migration features, fix a rollback all bug #3169

Conversation

leeallen337 commented Apr 28, 2019 • edited Loading

kibertoad commented Apr 28, 2019

kibertoad Apr 28, 2019

Choose a reason for hiding this comment

leeallen337 Apr 28, 2019

Choose a reason for hiding this comment

kibertoad Apr 28, 2019

Choose a reason for hiding this comment

leeallen337 Apr 28, 2019

Choose a reason for hiding this comment

kibertoad Apr 28, 2019

Choose a reason for hiding this comment

leeallen337 Apr 28, 2019

Choose a reason for hiding this comment

kibertoad commented Apr 28, 2019

kibertoad commented Apr 28, 2019

leeallen337 commented Apr 28, 2019

kibertoad commented Apr 28, 2019

leeallen337 commented Apr 28, 2019

kibertoad commented Apr 28, 2019

kibertoad commented May 19, 2019

leeallen337 commented May 20, 2019

andrewvsk Jul 16, 2019

Choose a reason for hiding this comment

leeallen337 commented Apr 28, 2019 •

edited

Loading