balderdashy / sails

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

Support for sails-migrations for versions > 0.10.1 #5152

Closed itayadler closed 5 years ago

itayadler commented 9 years ago

Me and @bluehotdog are trying to fix sails-migrations in order to support the new sails versions.

We're finding it difficult to run the migrations as we're required to register to the adapter a Waterline.Collection beforehand - This is a requirement that is weird for a migration. If we'll try to use registerConnection a second time it'll fail due a duplication error.

One way that could help us workaround this issue is to provide an updateConnection method that we can use to update later on the migration table (after loading sails with our schema_migration model), or to consider removing the dependency of the collections from the adapter.

We think that the adapters shouldn't hold any state or make any assumptions on the database schema, and should be more "functional", in that it will always goto the database to look for the current state.

BlueHotDog commented 9 years ago

Any news?

venticco commented 9 years ago

It would be nice to have this fixed.

tjwebb commented 9 years ago

I'm not clear on what the issue actually is. If someone wouldn't speaking a little slower, I'd appreciate help in getting up to speed on what's going on. thanks

BlueHotDog commented 9 years ago

Hi All, Let me try to give some background, some of it might be a bit emotional, but i'll try to keep things as pragmatic as possible. We're trying to make sails-migrations work for the new version of sails/waterline. Currently the process is less then good.

Working with waterline is hard, we're doing something on a fairly low level, and it feels like, unless all other libraries, the amount of work we need to do to make waterline "just work" is huge. The first problem is that, internally, the waterline behaviour, and interface changes drastically - but this is acceptable. The second problem is the way the adapter<->waterline communication is factored. There is state everywhere, and for migrations, which are stateless(and basically every database communication, except transactions, are stateless), and we need to manage this complexity.

Let me be more specific:

knex.schema.createTable(tableName, callback)

again, if you look at the source, there is no state. no connection managment, nothing like "collections" or the like.

My point is: there is no actual need for the collection abstraction, it's not part of the domain, it's not part of the ORM,(there also nothing the similar in the ActiveRecord word - which is probably the best ORM), it makes our job, as an external library, trying to work with waterline extremely complex. needing to keep track of state of the collections, without any real need.

Regarding the other comment. We need to load sails since we need some config, and we want the original sails project to manage the initialisation of waterline, we want to be a plugin for sails.

On Fri, Sep 12, 2014 at 10:28 AM, Travis Webb notifications@github.com wrote:

I'm not clear on what the issue actually is. If someone wouldn't speaking a little slower, I'd appreciate help in getting up to speed on what's going on. thanks

— Reply to this email directly or view it on GitHub https://github.com/balderdashy/sails-postgresql/issues/107#issuecomment-55370095 .

Danni Friedland +972-54-7764415

particlebanana commented 9 years ago

I'll chime in here but for other information see my comments on https://github.com/balderdashy/sails-mysql/issues/145.

The design pattern for making adapters responsible for keeping track of open connections is debatable but its that way in version 10 so any talk about changing it would be for version 11 which I don't see happening until things get a bit more stable. We chose it because it seemed like the best choice at the time and since adapters are much better about knowing how to work with things like replica sets and caches it made sense to pass that handling off to the adapter.

Adapters ARE simple lightweight abstractions that know how to communicate with a datastore. They keep a cached version of the schema in memory so that when a query comes in with normalized values, think dates, the adapter can know it needs to transform that into a sql compatible date string. The three examples you gave only work with SQL database so moving this logic into the ORM is easy. When you are adapter agnostic you can't assume anything about the schema so you pass in a normalized version of the data and allow the adapter to transform it as needed. Things like JugglingDb do the exact same things (https://github.com/jugglingdb/mysql-adapter/blob/master/lib/mysql.js#L217-L228)

It also keeps tracks of connections at the adapter level because Waterline allows you to use multiple connections on the same adapter. This is the largest piece that differs from other ORM's in your examples above: sequelize, knex, active record, etc. In waterline you can have two different remote postgresql databases and use them in the same project seamlessly. The postgresql adapter needs to know which connection to run a query on so when you call .create() on the adapter and pass in a tableName it looks up the active connection to use in order to perform that query.

For creating a table you have the adapter method define which works the same as your sequelize and knex examples above with the exception of the need for an active connection.

A collection in Waterline is simply an object that knows about it's schema and associations and how to connect to it's datastore. The ORM is just a dictionary of these objects. When you call User.create() it's the same as calling dictionary.user.create(). This is the same behavior as in 0.9. Again something we can talk about changing in the next version.

Sails offers hooks where you could use the same config loading as the ORM hook uses and spin up a Waterline ORM instance to do any migrations on then tear it back down. By using hooks you can define where in the Sails lift lifecycle you do this so you could tap into a spot before the ORM hook.

I'd argue migrations should be run independently of the application process though, which is what I assumed in the other issue when I gave the Waterline code, similar to rake db:migrate. Migrations should be something you manually run. The ORM hook in sails isn't doing anything fancy beyond the Waterline code I posted in the other issue. It globalizes the models but other than that it does the exact same thing. I'm open to suggestions for changing this behavior in the next version.

I can help with whatever is needed to get sails-migrations working with 0.10 but we can't completely rewrite the current ORM to get it working.

If anyone is interested Mike has an example of what a complete rewrite of Waterline might look like: https://github.com/mikermcneil/waterline2

cc @mikermcneil @sgress454

itayadler commented 9 years ago

I'll just clear something in what me and @bluehotdog are concerned about: keeping track of the open connections is fine, the dependency on the Collection is what breaks sails-migrations.

For instance if you have a project with 20 migrations and 5 models.. Let's say that in one of these migrations u added and removed some model.. Then sails-migrations will break, since it doesn't have a reference to that Collection anymore.

BlueHotDog commented 9 years ago

guys. we have like 2 issues with this problem. people are waiting and actively trying to make it work. we are to the point of thinking about switching sails-migrations to use sequelize(and got a working branch with that in about 1% of the time it took to debug waterline), really frustrating.

@mikermcneil please help.

mikermcneil commented 9 years ago

@BlueHotDog hey danni can you call me on skype? we should talk about https://github.com/mikermcneil/waterline2

BlueHotDog commented 9 years ago

Cool, lets try to do this around 21:00pm IST? Too late for me now..

— Sent from Mailbox

On Mon, Oct 6, 2014 at 11:03 PM, Mike McNeil notifications@github.com wrote:

@BlueHotDog hey danni can you call me on skype? we should talk about https://github.com/mikermcneil/waterline2

Reply to this email directly or view it on GitHub: https://github.com/balderdashy/sails-postgresql/issues/107#issuecomment-58086205

sailsbot commented 8 years ago

Thanks for posting, @itayadler. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

Thanks so much for your help!