sails: unable to set migration strategy per model

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.

About this issue

  • Original URL
  • State: open
  • Created 6 years ago
  • Comments: 32 (12 by maintainers)

Most upvoted comments

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 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 😃

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

@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.

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.

@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.

@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.

https://github.com/sailshq/waterline-utils/pull/89 https://github.com/balderdashy/sails-hook-orm/pull/19

demo screenshot

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?

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

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!