migrating from multiple files #993
Comments
|
The files are read in name order, and they are named: |
|
@Pomax It is considered a very bad practice to change migration files that have already been published and potentially used by other people. The correct approach is to create a new migration that introduces the needed changes. Migrations aren't the strongest point of knex unfortunately. Ideally you should also have a mechanism to recreate a database schema from a schema definition file, and not have to rely on migrations to always be the source of the schema. |
|
There are some deficiencies with the CLI, but personally I think they're superior to a schema file. Each to their own. |
|
@rhys-vdw I'm saying there should be a schema file and migrations. Migrations for quick and small changes of already existing databases, and the schema file for new installations. It's not very efficient to keep migrations around for a very long time if the schema has changed considerably over time and a lot of them are needed to recreate it correctly. |
|
I tried this with two files, |
|
Can you show us your migration files' contents? |
|
On a secondary note, I'm going to borrow a page out of Mike Hoye's book when it comes to documentation, for the earlier explanation on how migrations work: that is exactly the information needed. Instead of writing that information as an issue comment and asking to close this issue, please put that brief explanation in the migration section of the documentation: "to tell knex where the directory with migration files are, add |
|
Right, another undocumented feature of migrations is that the second argument to your exports.up = function(knex, Promise) {
return Promise.all([
knex.schema.createTable('publishedProjects', function(t) {
t.increments('id');
t.text('title').notNullable();
t.text('tags');
t.text('description');
}),
knex.schema //...
])
} |
|
I'm not sure what you mean by use |
|
that was just a placeholder. I'll happily submit a PR, but clearly @rhys-vdw already knew this and was in a great spot to preempt by filing that PR first =) |
|
I switched over the two files to use Knexfile: (The new file 1: Run result: There's also no mention which file things went wrong in, so that would normally make it harder to tell what knex is breaking on, except in this case there's only two migrations. |
|
It's probably not important, but Promise is already being passed as the second argument to both Also, the affected file is right there in the stack trace: |
|
missed that line. I am following the example from http://endpointsjs.com/tutorial/1 with your new recommendations on top. As for the promise being passed along, is that documented anywhere? |
|
What might be going on is that your database is in an inconsistent state, and the first migration was already recorded as completed, so it's not running again. I suggest you start over with a clean database and everything should work as intended. This is a weak spot for Knex's migrations, since they're not wrapped in a transaction, so if something goes wrong some things get applied and others not, which leaves the database in a messy state. |
|
Oh, and you should try the included docs first, which explain a more accurate process for creating migrations: http://knexjs.org/#Migrations-CLI |
|
Oh no, trust me, I read those, and they're not actual documentation; go through the text you linked me too, and you'll notice there is no information about how to write a migration file or what the call signatures are, how they're called, when, and how to deal with midway errors. There's some info on how to use the cli tool, that's about it. Heck, the text even talks about the props you can stick in a migration config object, then immediately moves on, never explaining any of those options. Real docs are incredibly needed here, still. With the current documentation, migrations are a mystery subject, even though it's one of the most important aspects of a database orm =) |
|
Cleaning wise: that's what the migrations table is for though? It should be telling knex which migrations already ran (in which direction and sequence). If we have no migration transactions, that's probably the absolute number one next bug to fix: a failed migration should never result in needing to wipe an entire database, as that has the potential to fatally break an already deployed system, simply because someone accidentally pushed code to master instead of to a staging branch, for instance. |
|
Currently, knex migrations aren't working as you expect. When something fails, the down migration won't run, and even if it did, it could try to undo things that don't exist, and that would also produce errors, so it's really just the lack of a transaction enveloping both the You don't need to wipe the entire database when there is a failed migration. I was just proposing that for easiness, since you seem to just be starting with that project. You can manually revert the database state without losing any data if you prefer. I know the docs are not very easy to follow, since I also struggled with this for a while, but all the information is there, just not clearly identified as such. It tells you how to create a a migration file, and by doing that you'll get a sample of how a a migration file looks like. Then you just have to use the appropriate schema builder methods. I'm sure that if you would be willing to contribute improvements to the docs, they would get reviewed and merged quickly. |
|
Hi @Pomax. Sometimes the database gets into an inconsistent state when a migration is partially applied. By default (with the latest version) migrations will be run in transactions, preventing this issue - so unless you've knowingly disabled it you should be fine. However it sounds as though you might be getting a race condition with exports.up = function(knex, Promise) {
return knex.schema // make sure you're returning here.
.createTable('publishedProjects', function(t) {
t.increments('id');
t.text('title').notNullable();
t.text('tags');
t.text('description');
})
.createTable('other table', function(t) {
}Also turn on |
|
Here's the problem: that's pretty much literally the code I was already using, for which @ricardograca told me to wrap things in Version wise, I'm using 0.8.6, which is the latest version as of this issue, as far as I can tell, and this always runs on an empty database for CI purposes. It's currently hooked into our PR testing setup, so updates to mozilla/publish.webmaker.org#166 based on advice in this issue end up getting built by travis in a clean environment, with the most recent Promise based migration yielding the following build failure: https://travis-ci.org/mozilla/publish.webmaker.org/builds/82093850 (https://travis-ci.org/mozilla/publish.webmaker.org/builds/82093850#L149 creates a brand new database every time CI runs, specifically to rule out problems relating to "bad old state") |
|
Hi @Pomax, I'm sorry to hear you're having a hard time. Please be aware that neither Ricardo nor I actually wrote Knex, nor this feature, we're essentially just users. I've already mentioned in another issue that I agree that there is a lack of documentation - sadly complaints alone will not ameliorate this situation. Unfortunately my contribution to knex is time limited to reviewing and merging community PRs and responding to occasional issues. I can't see anything wrong with your linked code. I think Ricardo's suggestion of dropping all your tables and starting from scratch, coupled with turning on debug logs, should go a long way to helping you work out what's going on. |
|
Yeah, sorry about my reactions. I've been trying to get unstuck on this for a while and have been venting the lack of progress as part of my unstuck process. Totally not okay, so apologies for that. The Travis instance always starts with an empty db, so I'm not sure how much more I can do there in terms of dropping tables, but I can try to put everything back to not using those promises and see where that gets me. Maybe there was something funny going on as rewrite-on-a-rewrite and that's been biting me. |
|
@Pomax @ricardograca From what I saw in the first migration file, using If you use bluebird's
So, for example, your |
|
And @Pomax, for what's it worth, I always put only one operation in each migration file. In other words if i'm creating a bunch of tables, I'll create one migration for each table. The same goes for alters and views. Sure you end up with more migration files, but having more files should also (in theory - see my comment on individual migrations #666) give you more granular control on rollbacks. |
|
@chrisbroome Yes, if you put all of them inside the same |
|
cool, let me try |
|
followed by file 2 with: leads to the initial creation and then succesful upward migration to extended schema. Thanks for the help, and again apologies for being so rude! |
|
Using Here's how to do it in one file and making sure dependencies are taken care of: exports.up = function(knex, Promise) {
return Promise.all([
knex.schema.createTable('blah', function(table) {
table.increments()
// doesn't depend on anything
}),
knex.schema.createTable('foo', function(table) {
table.increments()
// doesn't depend on anything
})
]).then(function() {
return Promise.all([
knex.schema.createTable('bar', function(table) {
table.increments()
// this depends on something from the first Promise block above
}),
knex.schema.createTable('baz', function(table) {
table.increments()
// this depends on something from the first Promise block above
})
])
})
}PS: You should read bluebird's documentation a few more times. |
|
cheers |
|
This is a great thread. I am sorry for the unfortunate experiments of @Pomax . It seems that much information herein should be included into the official documentation about migrations. Is it possible to contribute to the official site's documentation? Regarding the migration order, maybe we could use Django 1.7 excellent paradigm. A schema definition, which would allow in future the automatic creation of migration files. Migration files could feature something like dependencies in order to avoid the somewhat not-so-elegant way of filename ordering. |
|
Cheers @ricardograca that really helped 🙇 |
|
Looks like @Pomax did get his answer... closing the issue. Thanks everyone! |
|
@ricardograca That was INCREDIBLY helpful, thank you! An example like that in the documentation would be perfect. |
How do I ensure that knex migrates using multiple files in a reliable order, making sure to finish one file before moving on to the next? #148 suggests this the file processing order is based on file timestamp which would be insane, since that makes edits to files in order to fix isues with them (typos, refactors, etc) change the actual migration order. Is there an explicit ordering for files in a migration directory, or a way to specify them? I can't find anything about this in the docs.
The text was updated successfully, but these errors were encountered: