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

Exposing upToRun and downToRun without actually running migrations #551

Closed
1 of 3 tasks
RandomSeeded opened this issue Mar 13, 2018 · 7 comments
Closed
1 of 3 tasks

Comments

@RandomSeeded
Copy link

RandomSeeded commented Mar 13, 2018

I'm submitting a...

  • Bug report
  • Feature request
  • Question

Current behavior

Currently there is no ability to see what migrations will be run without actually running those migrations.

Expected behavior

Additional runnable commands upToRun and downToRun.

What is the motivation / use case for changing the behavior?

When deploying, we want to validate our data after running migrations. Doing so efficiently requires some amount of work (hashing collections) prior to running migrations. Given that most of the time we won't actually have any migrations to run, it would be extremely helpful if there was a method of knowing beforehand how many migrations will run so that we don't have to do this pre-computation.

I'm happy to work on a PR for this but am hoping not to maintain a separate fork. Assuming it met your quality standards, would you open to considering a PR for this functionality?

@RandomSeeded RandomSeeded changed the title Exposing toRun Exposing upToRun and downToRun without actually running migrations Mar 13, 2018
@wzrdtales
Copy link
Member

Doesn't --dry-run do exactly what you search for?

@wzrdtales
Copy link
Member

Or is it just about the number?

@RandomSeeded
Copy link
Author

RandomSeeded commented Mar 14, 2018

Hmmmm I'm not sure TBH; I've not used --dry-run before.

A quick bug report for something I came across when trying --dry-run out:

      var migrations = dbResults
        .filter(function (result) {
          return (
            result.name.substr(0, result.name.lastIndexOf('/')) ===
            internals.matching
          );
        })
        .map(function (result) {
          return new Migration(path.join(dir, result.name), internals);
        });

fails on .filter because dbResults is undefined, which is from the following code in db-migrate-mongodb:

    if(this.internals.dryRun) {
      return Promise.resolve().nodeify(callback);
    }

I'm not actually sure what the intended behavior of that last snippet is. If I change it to resolve([]) (which aligns with what seems to be intended behavior) then --dry-run works, but will print the output of all migrations regardless of whether or not they've already run.

If I just nuke that last snippet entirely --dry-run works as I'd like it to (and allows me to then just grep for [INFO] No migrations to run) but seems contrary to the intended behavior. 🤔

I feel like we're making solid progress here though, thanks for your help

@wzrdtales
Copy link
Member

Mh, gonna have a look at that. So you really just want to now if there are any migrations to execute and not the number of them?

@RandomSeeded
Copy link
Author

RandomSeeded commented Mar 14, 2018

Yup pretty much.

edit: though I think generally erring on the side of exposing more info better than exposing less. You can always turn a list of migrations to run into a boolean / count, but can't go the other way...

@wzrdtales
Copy link
Member

In that case sounds pretty much the same as #186, but some extended functionality. I could think of a command like check or something like that, that throws out a list of migrations to be executed by the command specified and in case of up migrations the already executed ones and the ones in plan.

So for the cli something like

db-migrate up --check
db-migrate down --check
db-migrate --check <- to just throw out the list of currently executed ones

For the programmable interface, I would think of something like that, which could make sense.

    dbmigrate.check().up();

If you're willing to contribute a PR that would be awesome!

@RandomSeeded
Copy link
Author

cool I should be able to look at it next week :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants