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

Ability to traverse sub directories #80

Merged
merged 1 commit into from
Oct 22, 2017
Merged

Conversation

miguelb
Copy link

@miguelb miguelb commented Jun 9, 2016

I wanted to be able to have a directory structure for my migration scripts since I'm working on a large project with many of migrations. Having a directory structure makes it friendlier for long-term and large projects. Some may argue that after a migration runs, it should be archived or moved away. Another option might be to have the database migrations run as a separate process or script. Perhaps, but allow me explain my environment.
Each service is packaged as a docker image with its model, code and all its migration files. Besides the migration files, they also contain seed scripts which is able to put the database to a "known" state for that service. All migrations and seed scripts are made to be re-runnable. This allows me take a docker image and update an environment, deploy it in a new environment or bring up a temporary environment to run smoke tests. Yes, it makes the service start up slower by a few seconds, but the benefits out way this minor lag. However, I plan to look into how to "mark" where the last run was and not have to traverse all the files but only where it left off at. For example, if the folders are structured as "year/month", it could look at the last script ran and derive which folders it should look at next.

My migration directory structure now looks like:

  • migration
    • 2015
    • 2016
      • 01
      • 02
        • 20160204000001-modify_foo.js
        • 20160204000002-create_bar.js
        • 20160204000003-remove_foo_bar1.js
        • ........
      • 03
      • 04

With this change, I added a new property called traverseDirectories. If this is set, it will recursively go through subdirectories, otherwise, it only searches the migration path (as before).

Run All Migrations:

var Umzug = require('umzug');
var sequelize = model.sequelize;
var umzug = new Umzug({
  storage: 'sequelize',
  storageOptions: {sequelize: sequelize},
  migrations: {
    traverseDirectories: true,
    params: [sequelize.getQueryInterface(), sequelize.constructor],
    path: __dirname + '/lib/database/migration'
  }
});

Run One Month Migrations:

var Umzug = require('umzug');
var sequelize = model.sequelize;
var umzug = new Umzug({
  storage: 'sequelize',
  storageOptions: {sequelize: sequelize},
  migrations: {
    traverseDirectories: false,
    params: [sequelize.getQueryInterface(), sequelize.constructor],
    path: __dirname + '/lib/database/migration/2016/02'
  }
});

This is currently working well in my environment and decided to share with others, in case anyone else finds it useful.

Thanks,
Miguel

@jukkah
Copy link
Contributor

jukkah commented Jun 9, 2016

Please add some tests. 👍

@miguelb
Copy link
Author

miguelb commented Jun 9, 2016

Cool. will work on it.

@miguelb
Copy link
Author

miguelb commented Jun 10, 2016

I leveraged all the awesome tests that currently exist to run on both flat and multiple directories. This allows you to continue to add tests and it will be executed on both directory structures. I got this idea after reading this https://github.com/mochajs/mocha/wiki/Shared-Behaviours.
I also updated the helper to add and remove directories.

Thanks,
Miguel

describe('down-directories', function () {
beforeEach(function () {
return helper
.prepareMigrations(3, {directories: [['1', '2'], ['1', '2'], ['1', '3', '4', '5']]})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this generate tmp/1/2.js twice?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. It checks if it exists and then adds it. This is done via the helper. These are directories and not file names. Every file you want to add (in this case, 3) should have a directory to add it too, otherwise, it will be in the root tmp directory. So, in this example there will be two files in 'tmp/1/2/' directory and one in 'tmp/1/3/4/5/' directory. The file and file names work as before, the change was to put them in directories.

@jukkah
Copy link
Contributor

jukkah commented Jun 12, 2016

Code looks good to me if tests are working right.

Please read also issue #33. There is some discussion related to this topic. I don't want to judge right now which one is better solution or is this PR subset of it.

This may also hit my issue #39. There would be regular migrations and optimized sequences of migrations = squashed migrations existing at the same time. We don't want to run them both = twice. This makes it breaking feature as I assumed earlier when assigning it to the milestone 2.0.

@miguelb
Copy link
Author

miguelb commented Jul 20, 2016

Issue #33 isn't exactly the same as this, since it references having multiple models and migrations folders. Here it is the same models but the migration scripts are in sub-directories making it more manageable for large projects with many migrations. It now handles recursing through subdirectories of the migration path.

Issue #39 will still work work as you describe it. Here is simply addressing the file structure of the migrations and not changing the migration names.

  // List of migrations squashed in this squash file.
  migrations: ['some-id', 'some-other-id'],

Will still work as described in that issue because the migration name ('some-id') doesn't change in regards to which folder its under. It still records the migration ran as 'some-id' and not a full path.

@miguelb
Copy link
Author

miguelb commented Jan 5, 2017

What is the verdict on the PR? Is there anything else you would like me to do or check?

Thanks,
Miguel

@fcanela
Copy link

fcanela commented Mar 8, 2017

Hello.

We could benefit from this feature. What is the current status? Any plan to integrate it?

Thanks

@jukkah
Copy link
Contributor

jukkah commented May 10, 2017

Why I didn't merge this when there was no conflicts? My bad. 😢

I'm publishing v2.0 soon (it's much smaller than I originally planned) and I decided to postpone this feature to v2.1 (or v3.0 if there are breaking changes). There is a lot of conflicts in this PR so I'll rewrite this for you if it's ok? I'm wondering if we could use globs instead of traverseSubDirectories as '**/*.js' just works.

@miguelb
Copy link
Author

miguelb commented May 10, 2017

If you have a milestone on which version you want to add this in, I can pull that version, fix any errors and update tests accordingly. Just let me know what you would like me to do or if you want to do it yourselves, I'm cool with that too. I'm here to help out however I can.

Thanks,
Miguel

@jukkah
Copy link
Contributor

jukkah commented May 10, 2017

Nice. 👍 If you are still interested in implementing this (again), there is branch v2.x to use as base. I'll merge that branch to master probably today but there should not be a lot of conflicts after publishing new major.

@jukkah
Copy link
Contributor

jukkah commented May 10, 2017

v2.0.0 is now out 🎉 so simply use master as your base branch.

@miguelb
Copy link
Author

miguelb commented May 10, 2017

Great, I will work on the changes.

@jimsimon
Copy link

This would make organizing complex migrations substantially simpler. Any ETA on it?

@iamvijaydev
Copy link

@miguelb, is it safe to use you PR branch for my requirements.

@miguelb
Copy link
Author

miguelb commented Sep 11, 2017

Sorry for the delay.
I updated my changes, fixed merge conflicts and updated tests. This is all set to be merged in whenever you like. Let me know if there is anything I can do to help.

Thanks,
Miguel

@miguelb
Copy link
Author

miguelb commented Sep 11, 2017

@iamvijaydev
We have been using my PR request for over a year in my company for dev and production without any issues.

Miguel

@miguelb
Copy link
Author

miguelb commented Sep 13, 2017

@jimsimon
Agree this allowed us to have a organize our migration directory in a way which makes sense to us. For example, below is a screenshot of our current directory structure. We do "year/month/script".

@PascalPflaum PascalPflaum self-requested a review October 3, 2017 20:34
@mlcloudsec
Copy link

any ETL on this please?

@PascalPflaum PascalPflaum merged commit 4983d13 into sequelize:master Oct 22, 2017
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

Successfully merging this pull request may close these issues.

7 participants