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

If sails is already loaded, do not load again #66

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

kmcgrath
Copy link
Contributor

This allows sails-migrations to be used within sails itself. It checks to see if sails is already loaded. If so, it does not do it again.

Then it is possible use sails-migrations directly to report on status and currentVersion from a Controller.

In the following example I also have the ability to migrate and roll back, but I realize you may not want to do that here, unless bootstrapping an application through a web interface.

/** 
 * MigrateController
 *   
 * @description :: Server-side logic for managing knex migrations
 * @help        :: See http://sailsjs.org/#!/documentation/concepts/Controllers
 */  

var vm = require('vm');
var sailsMigrations = require('sails-migrations');

module.exports = { 

  latest: function(req,res,next) {
    return sailsMigrations.migrate()
    .spread(function (batchNo, log) {
      if (log.length === 0) {
        res.json({
          message: 'Already up to date'
        }); 
      }   
      else {
        res.json({
          message: 'Batch ' + batchNo + ' run: ' + log.length,
          migrations: log 
        }); 
      }   
    })  
    .catch(next);
  },  

  rollback: function(req,res,next) {
    return sailsMigrations.rollback()
    .spread(function (batchNo, log) {
      if (log.length === 0) {
        res.json({
          message: 'Already at the base migration'
        }); 
      }   
      else {
        res.json({
          message: 'Batch ' + batchNo + ' rolled back: ' + log.length,
          migrations: log 
        }); 
      }   
    })  
    .catch(next);
  },  

  currentVersion: function(req,res,next) {
    return sailsMigrations.currentVersion()
    .then(function (version) {
      res.json({
        version: version
      }); 
    })  
    .catch(next);
  },  

  status: function(req,res,next) {
    return sailsMigrations.status()
    .spread(function (all,completed) {
      all = all || []; 
      completed = completed || []; 

      res.json({
        all: all,
        completed: completed
      }); 
    })  
    .catch(next);
  }   

};  

@RWOverdijk
Copy link
Collaborator

@kmcgrath Are you sure that the options don't have to be set?

@RWOverdijk
Copy link
Collaborator

@mikermcneil Maybe you can advise me on this one :) It looks good to me, but you probably know better.

@kmcgrath
Copy link
Contributor Author

@RWOverdijk options are set only if sails is not loaded, which still happens here. If sails has been loaded they should already have been set. Does that address your question?

@RWOverdijk
Copy link
Collaborator

@kmcgrath Almost! I just wonder if migrations perhaps needs some settings that the running instance perhaps doesn't have

@RWOverdijk
Copy link
Collaborator

@kmcgrath Is this PR still relevant to you?

@kmcgrath
Copy link
Contributor Author

kmcgrath commented Sep 1, 2016

It is. We use this to provide an admin interface that can be used to see migration status as well as bootstrap new applications through a web interface/API.

@RWOverdijk
Copy link
Collaborator

To me this PR makes sense. I haven't tested it yet, so I should. Have you tested this with and without running it yourself? If so, could you perhaps tell me how you tested it? This lib needs more tests, I'm aware of that.

@kmcgrath
Copy link
Contributor Author

kmcgrath commented Sep 1, 2016

We have a DevOps team running it in production for an internal application. I'm actually not on that project anymore but I help out when needed. I can try to get one of them on this PR, if needed.

@RWOverdijk
Copy link
Collaborator

@kmcgrath Seeing how this is a tool used in production by multiple people, I'd very much like that yes. Better safe than sorry. :)

@RWOverdijk
Copy link
Collaborator

@kmcgrath Would you mind rebasing this? I just merged the other PR, which conflicts with this one.

… into feature/look-for-sails

Conflicts:
	lib/sails-migrations/helpers/sails_integration.js
Conflicts:
	lib/sails-migrations/helpers/sails_integration.js
@kmcgrath
Copy link
Contributor Author

kmcgrath commented Sep 3, 2016

Sorry for the multiple commits we had a branch with two already merged. I resolved then merged with that one.

It boils down to one condition statement that checks for sails.hooks. If that exists then it does not run sails.load because it is already loaded.

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.

2 participants