unable to set migration strategy per model

[chestercharles]

Sails version: 1.0.2
Node version: 8.9.4
NPM version: 6.1.0
DB adapter name: sails-disk
DB adapter version: version that ships with sails 1.0.2
Operating system: Windows 8.1 Enterprise

---

Howdy,

I've started to use Sails v1 for new projects after using v0.12 for many previous projects. In v0.12, I set a migration strategy I wanted to use for my models in config/models.js.

/**
 * Default model settings
 * (sails.config.models)
 */
module.exports.models = {
    migrate: 'drop',
}

For a given model, I could set a different migration strategy in that model file (eg. api/models/Kitten.js) that would override what was in config/models.js.

/**
 * Kitten.js
 *
 * @description :: A model definition.  Represents a database table/collection/etc.
 * @docs        :: https://sailsjs.com/docs/concepts/models-and-orm/models
 */
module.exports = {
    migrate: 'safe',
    attributes: {
       
    }
}

When the server lifted, it would honor this model-specific migration strategy, and leave my kittens data intact.

Is this no longer the default behavior in v1.x? I do not see this on the list of breaking changes when upgrading to v1.

Steps to reproduce (or see this repo):

1. Generate a new app: sails generate new testapp1
2. Create a kitten model: sails generate model kitten
3. Create a puppy model: sails generate model puppy
4. Set migrate: 'drop' in config/models.js
5. Start sails with node app.js
6. Hit http://localhost:1337/kitten/create a couple times to generate some kittens.
7. Shut down sails
8. Add migrate: 'safe to api/models/Kitten.js
9. Start sails with node app.js
10. Go to http://localhost:1337/kitten... we've lost all the kittens.

[sailsbot]

@chestercharles Thanks for posting, we'll take a look as soon as possible.

---

For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

[johnabrams7]

@chestercharles The Sails v1.0 breaking changes log states, "The experimental create auto-migration scheme is no longer supported. It is highly recommended that you use a migration tool such as Knex to handle migrations of your production database." Hope this helps!

[chestercharles]

@johnabrams7 , I don't think the "create auto-migration scheme" is what I'm trying to use, here. I just want to be able to use different migration strategies for different models.

For example, in my case I have a User model which uses a datastore that is actually a web service that pulls data for employees at my company. I want to configure the model so that it always uses the safe migration strategy

The Model Settings documentation state they "can be specified on a per-model basis by setting top-level properties in a model definition". I would assume this includes the migrate setting.

[chestercharles]

After digging into the sails-hook-orm, it appears that auto-migrations are performed using whatever migration strategy is set in sails.config.models.migrate:
https://github.com/balderdashy/sails-hook-orm/blob/master/lib/build-ontology-and-run-auto-migrations.js#L132

    // Now that all relevant attributes have `autoMigrations` properties set, go ahead
    // and perform any requested migrations.
    WaterlineUtils.autoMigrations(sails.config.models.migrate, freshOntology, function(err) {
      if (err) { return done(err); }

      return done(undefined, freshOntology);
    });

I would expect that the orm hook would also inspect the migrate setting of each model, and perform the corresponding auto migration strategy for that model.

[raqem]

Hi @chestercharles,
I am looking into your issue more (thanks alot for adding a repo so I can easily test it out!) The first thing I am noticing is that in your config/models.js you have migrate set to drop. What that means is that every time you shut down Sails you are telling it to drop your data! So all your kittens are lost because its wiping your data clean!
I think you want to start with your migrate: 'alter' and see where that gets you!
But note that this migrate setting is only to be used locally in test environmants. Like it states in the models file once you are in production Sails will treat all models as if they are set to safe.

Also here are the official docs for migration for easy access:
https://sailsjs.com/documentation/concepts/models-and-orm/model-settings#?migrate

[chestercharles]

Hi @raqem, thanks for looking into the issue.

My confusion is not about what the different migration strategies do, but rather why Sails no longer supports migrate as a top level model property.

The documentation says:

Model settings allow you to customize the behavior of the models in your Sails app. They can be specified on a per-model basis by setting top-level properties in a model definition, or as app-wide defaults in sails.config.models.

However, in Sails v1 this is not the case - the migrate setting cannot be set on a per-model basis. It can only be set as an app-wide default in sails.config.models.

In Sails v0.12.9 I was able to have a different migrate setting per each model - this was very helpful when I had different models using different connections (datastores) that may or may not be read-only. I've modified my initial repo to include another puppy/kitten app generated with Sails v0.12.9 to demonstrate this.

I could certainly be misunderstanding something, but this appears to be a breaking change?

[chestercharles]

Hi @raqem. Does my previous question make sense? Have you and your team had a chance to take a look at this yet?

[krewx]

I have the same issue that @chestercharles it's commenting. I have the inverse situation, my db is so big that my migration it's on safe and i want to alter or drop certain models but keep safe migration for the rest of the models.

[chestercharles]

I've been working around this by making the drop and define methods fail silently in my custom adapters for read only data:

define: function (datastoreName, tableName, definition, done) {
    logger.warn(`Adapter method ('define') not implemented in this adapter.`)
    return done();
  },

However, this is a sloppy hack and would not work in @krewx 's situation.

Does Sails plan to support setting migration strategies per model?

[raqem]

Hi @chestercharles,
So sorry for the long delay! I am going to tackle someone today until I can get some kind of satisfactory answer for you before the holiday!

[chestercharles]

@raqem thanks! But dont hurt anyone on my account! 😳

[raqem]

Hey @chestercharles @krewx,
Its official, Sails V1 does not support setting each model to its own migration schema.

It was there in the docs all along but apparently a little too easy to read over since I swear I read that page a half-dozen times.

My understanding is that this was done to avoid having a situation where associated models have different migration strategies and by one slip up become a massive tangle and database death-heap.

[chestercharles]

@raqem I did see that note in the docs, but I thought it was just a suggestion.

Here is my case why you should consider supporting it:

In most real life situations, an application is going to need to interact with more data sources than just the core CRUD models, eg. like 3rd party APIs.

Sails/Waterline is awesome because it provides an adapter specification (and generator) so that I can integrate these different datastores into my applications and interact with them using the sails model APIs.

However, if I need to use a datastore that is read-only (say, to integrate data from an external system), without being able to specify migration strategies per model (or at least per datastore) I can no longer use the migration feature of sails.

Also, I can no longer use the sails-disk adapter for quick-rev development since I can't really manually migrate models with sails-disk. And for any relational databases I want to use, I have to manually create all the tables.

I feel like dropping support for this feature undermines the benefits of the adapter-based approach of waterline. It is going to be too much of a burden to use Sails to develop applications that need data from external systems via waterline adapters.

My two cents - thanks for your consideration and happy Thanksgiving :)

[alwaysAn0n]

The option to set the migration strategy on a per-model basis was incredibly useful. Maybe using another migration tool is safer but I'm not going to do it. I keep coming back to Sails because the ORM is badass but this change makes the ORM significantly less useful. Please bring it back

[raqem]

Hi @alwaysAn0n,
Thanks for the love! And for your thoughts on the issue. The more people who share ya'lls feelings on the matter the more the Sails team will keep this in mind.
I am sorry I cant offer any promises but we are hearing ya!
~Cheers

[raqem]

Hey ya'll @chestercharles, @krewx, @alwaysAn0n
Brought this up to @mikermcneil in a meeting. This is a totally valid use-case (thanks for providing one so we can get into your headspace!) 
The biggest blocker for making this happen is associated models.
Since Waterline auto creates a join-table what would the migrate setting be for that table if the two associated models had different migrate strategies? 

If anyone is interested in giving this a good think and making a PR Mike is interested in reviewing it. But for now, it’s just not on the docket for the core team.

[alwaysAn0n]

Thanks for the quick response Sails team!

The biggest blocker for making this happen is associated models.
Since Waterline auto creates a join-table what would the migrate setting be for that table if the two associated models had different migrate strategies?

That's a totally valid concern. I would do it like this. When trying to auto-migrate a particular model, check if it actually does have associations. If yes, refuse to migrate ( and log a warning ) unless all associated models share the same per-model migration strategy as the model in question. If the model in question doesn't have associations, migrate using the specified per-model migration strategy.

As for submitting a PR, I might be able to do that. Would this only require a change in waterline-utils or does it touch sails-hooks-orm and/or the adapters?

[mikermcneil]

@alwaysAn0n all automigrations logic is driven by sails-hook-orm, but it does use some utilities in waterline-utils. So this is something that'd mostly live in waterline-utils, I think, but it'll definitely involve npm link-ing both of those packages, and might also involve changes in sails-hook-orm that I'm not thinking of right now. (tldr; yes I think only waterline-utils, but be sure to link and test in the context of a sails app to make sure there aren't any unexpected surprises. automigrations have been notoriously difficult to test)

[phillipweston]

I'm experiencing the same thing. Seems counter intuitive to force migration policy at the entire app level, as it removes our ability to integrate legacy databases with stuff we're newly developing. I'm looking at:

https://github.com/balderdashy/sails-hook-orm/blob/master/lib/build-ontology-and-run-auto-migrations.js

and

https://github.com/sailshq/waterline-utils/blob/master/lib/auto-migrations/index.js

Am I correct in observing that it determines the automigration policy at the app level via sails.config.model.migrate and also on a per adapter level? So a sails-sqlserver would be able to migrate differently than a sails-mysql? Wondering if the reason we are having an issue is because both databases use the same sails-mysql adapter (orm)? If that is the case, then should we be able to delineate separate per adapter migrations by including the datastore name?

Looking at this in particular:

  switch(strategy){
    case 'alter': runAlterStrategy(orm, cb); break;
    case 'drop': runDropStrategy(orm, cb); break;
    case 'safe': runSafeStrategy(orm, cb); break;
    default: return cb(new Error('Strategy must be one of: `alter`, `drop`, or `safe`.'));
  }

It looks like orm is a single object that encapsulates every adapter?

Is there a reason we can't inspect the migrate type on the WLModel?

We don't even really need to worry about migrations happening on our new application database, all for that, but being able to specify migrate: 'safe' for entire legacy databases we just want to be read only is the main goal.

This is the only thing that is stopping me from wanting to move forward with Sails, so I'm very motivated to help make it happen. Going to npm link and see if I can propose a solution.

[phillipweston]

@mikermcneil made a couple PRs to address this. I imagine you'll want some things changed, but would like your feedback and to know what I need to change or do get it in. Thanks!

This update allows specifying migrate per datastore.

sailshq/waterline-utils#89
balderdashy/sails-hook-orm#19

demo screenshot

[phillipweston]

Hey Mike, I also wanted to thank you and the team for making Sails, and look forward to using it to make life easier!

[mikermcneil]

@atomiccc thanks! I like (a lot) the idea of it being per-datastore rather than per-model (makes way more sense) -- for consistency, I think that'll mean updating the docs to change it from a model setting to a standard datastore config key.

The only problem is that this way is potentially a breaking change, depending on how we do it. And realistically, the right way to do this is with a breaking change, since that way we're not adding a bunch of deprecation warnings and stuff we have to unwind later.

I'd like to hear some other thoughts too, just to get a sense of whether this change would obviously mess anyone up in some way we're not anticipating.

[Kontributer]

I’m not sure why this would be a breaking change. It seems like there should be a default (app-wide) migration strategy if one isn’t specified. That’s the current behavior and nothing to deprecate. This feature should only build on that; use the strategy added to the adapter, if specified.

Ideally, though, setting the strategy in the adapter is still too wide. I don’t want to discount the concerns and complications previously mentioned, but the migration strategy should be a per-model setting.

I have models that are simple ORM interfaces to database views. For instance, I’ve been granted access to an HR employee view of a database I don’t administer. I don’t have permission to drop those views, nor would I want to. They are read-only data, which should also not be replicated in another database to honor data governance policies.

In other databases, which I do have write access, I don’t want to accidentally drop tables that are managed by other applications. For instance, let’s say I’m building a notification interface to notify user lists of varying system events. This notifier queries varying database looking for conditions that trigger a notification and build a subsequent report with more details of the data. I may want to log that in an area of that database (maybe in a log db schema), without being able to drop those main tables. That requires different migration strategies w/in the same database — unless the advice is to have two adapters/connections to the same database (read stream vs write stream).

---

I know Mike may not be looking for parroting the original idea, but wanted to reinforce that there are countless cases where I have needed to perform migrations on a per-model basis and it makes sense to be able to set migrations at the three layers:

• app
• database
• model

Since only the app is currently available, I’d be fine with using the database as the next stepping stone for that capability.

I don’t want to get rid of the app ability as there are lots of projects that are also self contained and it makes sense to include it for backwards compatibility (and prevent a breaking change).

[raqem]

Hi @Kontributer Thanks alot for your thoughtful feedback.
I am going to keep this issue open to more community feedback mostly to address @mikermcneil concern

I'd like to hear some other thoughts too, just to get a sense of whether this change would obviously mess anyone up in some way we're not anticipating.

But we have counted you as a vote for 'in favor of changing how strict migration settings are'

[vikas-0]

this is not useful as long as there is no feature to automatically create migration such as django.

[whichking]

Hey, @vikas-0! We're continuing to monitor community opinion on this topic, and we appreciate your feedback!

[TheAdamGalloway]

Hey guys - just wanted to add my support for a model-level setting for migration strategy. I'm working with user data in an external database whereas all my other data is in its own database. I can't drop the external tables so this is giving me issues at the minute. If anybody has any workarounds I could use please let me know!

[alwaysAn0n]

Hey guys - just wanted to add my support for a model-level setting for migration strategy. I'm working with user data in an external database whereas all my other data is in its own database. I can't drop the external tables so this is giving me issues at the minute. If anybody has any workarounds I could use please let me know!

The workaround I used was temporarily commenting out all models I didn't want migrated then lifting the app. If the ORM doesn't know the models exist in the datastore, it won't fuck with them.

[TheAdamGalloway]

Hey guys - just wanted to add my support for a model-level setting for migration strategy. I'm working with user data in an external database whereas all my other data is in its own database. I can't drop the external tables so this is giving me issues at the minute. If anybody has any workarounds I could use please let me know!

The workaround I used was temporarily commenting out all models I didn't want migrated then lifting the app. If the ORM doesn't know the models exist in the datastore, it won't fuck with them.

Yeah this is what I ended up doing also. It's a bit of an annoying warkaround though.

[danielsharvey]

I just wanted to add my support for a model-level and/or adapter-level setting for migration strategy. I'm looking at a REST adapter, which does not want migrations performed ever.

[vidueirof]

Hi, I'm in a similar situation. In my case y develop a json based adapter to use with preloaded data that will not change. I have one or two models working with it and the rest of the them with mongo.
I'm writing some tests so I'm overriding the global migrate to drop so each test run with an empty db except for the two models with preloaded data.

Right now my only option is to change my drop & define to prevent the erase, but it would be great to override the migration strategy model based.

Thank all the people working hard to make sails a great framework!!! And if I can in the near feature I will be releasing the db adapter (made on top of low db).

[DominusKelvin]

Hey, all. I'm working on Sails Content which will definitely benefit from having a per-model or per-datastore migration. I am going to take a stab to see if I can land a PR for this.