sort lfs.dir result; bail out if file open fails

ostinelli / gin

A LUA fast, low-latency, low-memory footprint, web JSON-API framework with Test Driven Development helpers and patterns.

MIT License

238 stars 35 forks source link

sort lfs.dir result; bail out if file open fails #10

Closed istr closed 9 years ago

istr commented 10 years ago

lfs.dir does not necessarily sort by filename, so we need to sort to process in correct order if applying multiple migrations at once
if iopen fails, bail out with an error and show what went wrong

ostinelli commented 10 years ago

Why do you think that CommonHelpers.module_names_in_path needs to return modules names ordered alphabetically?

istr commented 10 years ago

Because a migration may depend on a previous migration so it is vital to execute them in order. Example: current migration: migrations/20140622141203.lua migrations/20140625174238.lua creates a table: CREATE TABLE foo (id SERIAL, bar INTEGER); migrations/20140625185527.lua alters that table: ALTER TABLE foo ADD COLUMN op INTEGER;

Now you want to apply these two migrations on a host that has 20140622141203 as its last migration. If these are executed in arbitrary order, migration may fail. The same applies to rollbacks.

It could be sufficient to sort within Migrations.migration_modules only, but more often than not it is preferable to have deterministic rather than arbitrary behaviour, so I just put it into the helper function.

istr commented 10 years ago

Note: you may pull https://github.com/ostinelli/gin/pull/9 as well, because it adresses the same problem and provides the option to run specific migrations.

ostinelli commented 10 years ago

Ok thank you. #9 does not pass tests. I've asked @leite to fix tests and I'll merge that one in.

You have 2 pull requests with the same issue, I've closed #11. I'm assuming we'll wait to see if @leite's one gets merged in depending on tests being fixed. Thank you.

istr commented 10 years ago

Ok, closing in favour of pull request https://github.com/ostinelli/gin/pull/9 brought in by @leite.

istr commented 9 years ago

I merged in current master branch into istr:sortmigrate because @leite did not follow up on his pull request. My version does the work and passes the busted test.

ostinelli commented 9 years ago

Thank you @istr will review soon.

ostinelli commented 9 years ago

Hello @istr, can you please fork out of current master and generate a single squashed commit with all your changes. Will apply when done.

As you can see some of this is already in master: f77b0ecf3eefafc90dd49256907a3a589b18a0ae

Thank you.

istr commented 9 years ago

Ok, so hopefully I got it right with git this time... ;-) My "bread and butter" RCS is http://fossil-scm.org which is way easier to handle. See pull request from my branch "orderedmigration".