balderdashy / sails

Realtime MVC Framework for Node.js
https://sailsjs.com
MIT License
22.83k stars 1.95k forks source link

Multiple named adapter connections using the same module #939

Closed particlebanana closed 10 years ago

particlebanana commented 11 years ago

When you have multiple adapters named in config/adapters.js that use the same module they will all get squashed together into a single object under the module name in sails.adapters.

config/adapters.js

module.exports.adapters = {
  "default": "foo",

  foo: {
    module: 'sails-mongo',
    url: 'some connection string'
  },

  bar: {
    module: 'sails-mongo',
    url: 'some connection string'
  }
}

Will get built as the following internally:

sails.adapters

{
  'sails-mongo': {
    identity: 'sails-mongo'
  }
}
alexmuro commented 11 years ago

Thanks for posting this, I believe the bug lives in here:

https://github.com/balderdashy/sails/tree/master/lib/hooks/orm

I am taking a look at it now, I haven't contributed to the project before but if I figure it out I will try to put together a pull request.

alexmuro commented 11 years ago

I am a bit stumped here: in https://github.com/balderdashy/sails/blob/master/lib/hooks/orm/index.js console.log(adapters[sails.models[model].adapter].config) right after var adapters = ormUtil.overrideConfig(model); on line 123 if will return the correct configuration as specified in you sails model.

however if you console.log(model,sails.models[model].adapter.config); right after sails.models[model] = collection; on line 14 all the configs will be the same.

it seems like the error is happening on the instantiation of the the model. Could this be some kind of scope error due to
var adapters = ormUtil.overrideConfig(model); on line 123

Any thoughts or help would be greatly appreciated.

mikermcneil commented 11 years ago

Guys, I'll look at this on Tuesday on the plane home-- it's a bit confusing because of the overloaded adapter terminology.

Think of it this way:

You have many connections to different services/databases/things in your app, e.g. Dell's Sales database (mysql) and Dell Marketing mysql database, and Alex's twitter account, and sailsjs twitter account, and Alex's Mandrill account.

Each connection might use the same or different adapters, and might have different config, e.g. apiKey, host, port, and so on. These connections are set up in your config/adapters.js file, along with the default connection for models, and then each model can override that default with one or more connections. For convenience, models should also be able to reference adapters directly.

That all sounds very straightforward, but unfortunately, the nomenclature is currently rather confusing :) So on Tue night or so, I hope to go in and clean things up, change the name of the config object for the next minor version (everything will be backwards compatible).

Any ideas/help more than welcome! Thanks for digging in to this :+1: Mike

alexmuro commented 11 years ago

Hey Mike,

Thanks for the response, I am unlikely to do a better job cleaning this up than you will but I have moved on from where I was stuck earlier today. I just want to get it to work for my own sails project, so I will probably keep hacking at it, if I figure out anything useful I will be sure to post it back here.

While there is some ambiguity between adapter and adapters and sails.adapters and so on, the code in here is generally awesome and easy to read and I am learning a ton by going through everything and figuring out how the framework is functioning. So thanks for all your great work on this project.

Alex

alexmuro commented 11 years ago

So I believe, at least in my case this problem has to do with node's require(), caching whichever adapter module you are using, for me sails-postgresql.

No matter how many configs you have for a particular adapter you only ever have one instance of the adapter because of the way require works. Therefore whichever model is the last one to be instantiated sets the configuration of the adapter module, the adapter then has that config whenever any subsequent model, which may have had a different config, uses it.

I came upon this by creating a waterline.collection in one of my controllers. In this controller i also required sails-postgres and before instantiating my collection I added the line:

postgres.config.database = 'not_the_default_database';

When I started my app, my models all accessed the default database and returned data. However when I access the route which calls this particular controller method, it would work and get data from 'not_the_default_database'. However after visiting that route, all the other routes were now reading from 'not_the_default_database' and would return [].

I am still new with node, so I could be drawing incorrect conclusions as to what is going on, or just configuring my project incorrectly, but I hope this is helpful.

I am able to get around this by using postgres's dblink extension and Waterline.Collection.query for now. (http://blog.cuerty.com/posts/installing-dblink-in-ubuntu-1204-and-postgresql-91.html) (http://stackoverflow.com/questions/17683015/how-to-perform-sql-joins-in-sails-js-waterline)

wyoumans commented 11 years ago

+1 Would love a workaround for MongoDB

particlebanana commented 11 years ago

Mike has this worked out in Sails now. I don't think it's been pushed out to NPM yet though.

wyoumans commented 11 years ago

thanks. I'll check it out.

wyoumans commented 11 years ago

This does seem to be fixed on master according to the commit history, but I think it is now expecting a different configuration in config/adapters.js. Is there any updated documentation?

sgress454 commented 10 years ago

FYI adapters.js is now connections.js in v0.10, and this problem is fixed.

aminjam commented 10 years ago

+1. Looking forward to the fix for this. As we are running to the same issue.

xiwan commented 10 years ago

Hi All, I mentioned a similar bug a week ago, and gave a temp hack solution:

https://github.com/balderdashy/sails-mongo/issues/175

mickyginger commented 10 years ago

Getting this issue in v0.10.4

mickyginger commented 9 years ago

Still getting this issue in v0.10.5

insidewhy commented 9 years ago

It seems also settings in config/env/*.js under "connections" will also affect the connections used. Is it recommended to put the settings there or in config/connections?

javifr commented 9 years ago

+1 Getting this issue in v0.10.5

insidewhy commented 9 years ago

Man this is disappointing. How can such an obvious and fundamental bug exist for so long. Is nobody using sails with multiple environments? I'm working somewhere that's actually decided to put it into production...

particlebanana commented 9 years ago

@mickyginger @nuisanceofcats @javifr can you explain a bit about what you are seeing? I wasn't aware a connection issue existed.

insidewhy commented 9 years ago

@particlebanana I don't have anything add to the very first comment which illustrates this issue very well.

particlebanana commented 9 years ago

@nuisanceofcats I just ran that example with a fresh sails app and everything works as expected, I used mysql because I didn't have remote mongo instances available but that shouldn't change anything about the connection logic in sails/waterline.

The config file is now named connections in 0.10.x so you should have something that looks like:

module.exports.connections = {
   mysql_a: {
      adapter: 'sails-mysql',
      host: 'some_host',
      user: '',
      password: '',
      database: 'foo'
  },

  mysql_b: {
      adapter: 'sails-mysql',
      host: 'some_host',
      user: '',
      password: '',
      database: 'bar'
  },
}
jeromewir commented 9 years ago

Also getting this issue in sails 0.11 & sails-mongo 10.5. When I add two adapters with sails-mongo in config/connections.js, the second one is always used.

GBGuinan commented 9 years ago

I have the same issue trying to use two sails-mongo adapters in connections.js, can anyone help

besarthoxhaj commented 9 years ago

I got the same issue here :( with multiple mongo connections looks like

// models.js
module.exports.models = {
    connection: 'mongoTest',
    migrate: 'safe'  
};
// connections.js
mongoProd: {
    adapter: 'sails-mongo',
    host: process.env.MONGO_HOST || 'localhost',
    port: 9999,
    user: process.env.MONGO_USER || null,
    password: process.env.MONGO_PASSWORD || null,
    database: 'foo-prod'
},

mongoTest: {
    adapter: 'sails-mongo',
    schema: 'true',
    url: 'mongodb://user:xxxx@kahana.mongohq.com:1234/fooBar',
},

For some reason sails tries always to connect to mongoProd @particlebanana @mikermcneil I love sails guys, and this problem is pretty annoying since we are close to go to production. Is there at least some work around for that?

izaakrogan commented 9 years ago

+1

particlebanana commented 9 years ago

@jeromewir @GBGuinan @besarthoxhaj @izaakrogan hey guys I'm not seeing this. You have your model config setup correctly right? If you are using sails make sure you have the following in config/models.js:

module.exports.models = {
  migrate: 'safe',
  // The name of the connection to use
  connection: 'mongoTest'
};

If you aren't using sails then define the connection name in the model as shown here: https://github.com/balderdashy/waterline/blob/master/example/express/express-example.js#L63-L64 the only way it would use a random connection is if one wasn't defined and it needs a default connection.

besarthoxhaj commented 9 years ago

@particlebanana thanks for the fast reply. Unfortunately it still doesn't work. I will try to replicate the problem in a new sails app and see if I make any progress

randallmeeker commented 9 years ago

If its a difference between production and development you can put your definitions in config/env as a solution. http://stackoverflow.com/questions/28069677/handling-database-environment-configuration-in-sails-js/28080407#28080407

However, if you need to use both in the same environment, then I'm stumped.

particlebanana commented 9 years ago

@besarthoxhaj do you have anything in config/local.js? That would override any other connection settings.

jeromewir commented 9 years ago

I was using env/development.js & env/production.js with both connections in connections.js and I had the problem with sails.

Thanks to @randallmeeker solution, its works now. I just had to put my different configurations in each files.

Thanks for fast replies !

besarthoxhaj commented 9 years ago

@particlebanana @randallmeeker thank you guys! I solved it by creating different files in the env folder.

config/
    env/
        production.js
        staging.js
        testing.js

Each of them looks something like this:

module.exports = {
    connections : {
        mongoProd: {
            adapter: 'sails-mongo',
            host: process.env.MONGO_HOST || 'localhost',
            port: 27017,
            user: process.env.MONGO_USER || null,
            password: process.env.MONGO_PASSWORD || null,
            database: 'abc-prod'
        }
    },
    models: {
        connection: 'mongoProd',
        migrate: 'safe'
    },
    port: process.env.PORT || 80
};

Thats it! Thanks for the help!:)

p.s. @particlebanana my config/local.js looked like this:

module.exports = {
    port: process.env.PORT || 1337,
    environment: process.env.NODE_ENV || 'development'
}

I don't know if this was part of the problem.

ccurtisj commented 9 years ago

@besarthoxhaj Thanks, worked for me in 0.11.0. Was driving me nuts deploying to heroku!