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

Allow to migrate all migrations with one "mark_migrated" call #125

Merged
merged 4 commits into from
Sep 30, 2015

Conversation

HavokInspiration
Copy link
Member

Added a special value all for the version argument of the mark_migrated command.
It allows to mark all migrations found (based on the other arguments) as migrated in one call.
Everything is done in one transaction. If one the marking as migrated operations raises an exception, the entire process is cancelled and the transaction rollbacked.

I went with all instead of * because the * raised a TooManyArguments exception (I could figured out why though).
And all is closer to what we have in bake for instance.

The tests will fail because I'm waiting for cakephp/phinx#641 to be reviewed, tested and merged (to be able to merge #123 after the build passes).

Refs #124

@lorenzo
Copy link
Member

lorenzo commented Sep 15, 2015

Seems like tests are failing

@dereuromark
Copy link
Member

But is * now behaving like any other input? That just says not found?
Or will it still just add 0 as version in the DB creating an SQL error?

@HavokInspiration
Copy link
Member Author

@lorenzo Yes, that is expected, see last paragraph of my initial post.

@dereuromark The problem is that with bin/cake mark_migrated *, I get an exception thrown by the Symfony Console Component on my Ubuntu workstation.
With which OS are you using the Shell ?
The difference of behaviour might come from there.

Either way, I'll take another look to see what I can do with that * problem.

@HavokInspiration
Copy link
Member Author

@dereuromark I took a look at that * issue. I tested it on a debian vagrant box and it seems that if I use bin/cake migrations mark_migrated * the * has a special meaning in unix : it will return everything present in the folder from where I launch the command. This is a debug of $_SERVER['argv'] on Symfony\Component\Console\Input\ArgvInput constructor :

[
    (int) 0 => '/vagrant/app31/bin/cake.php',
    (int) 1 => 'migrations',
    (int) 2 => 'mark_migrated',
    (int) 3 => 'bin',
    (int) 4 => 'composer.json',
    (int) 5 => 'composer.lock',
    (int) 6 => 'config',
    (int) 7 => 'index.php',
    (int) 8 => 'logs',
    (int) 9 => 'phpunit.xml.dist',
    (int) 10 => 'plugins',
    (int) 11 => 'README.md',
    (int) 12 => 'src',
    (int) 13 => 'tests',
    (int) 14 => 'tmp',
    (int) 15 => 'vendor',
    (int) 16 => 'webroot'
]

So there is nothing I can do on that side.
But for other OS that don't behave like this, to prevent the error you mentionned in your issue ticket, I can add a || $version === '*' in the test to also support it.
But I won't communicate on it since it does not work the same for everyone.

@dereuromark
Copy link
Member

I agree, we can catch it to prevent the error but we should stick to the documentation for the intended all.

@HavokInspiration
Copy link
Member Author

@dereuromark Done. We just need to wait for the PR that will fix the tests for be merged on phinx. If it takes too long I'll try to do something on our side as a temporary fix.

Added a special value "all" for the "version" argument of the "mark_migrated" command.
It allows to mark all migrations found (based on the other arguments) as migrated in one call.
Everything is done in one transaction. If one the marking as migrated operations raises an exception, the entire process is cancelled and the transaction rollbacked.
@HavokInspiration
Copy link
Member Author

Branch rebased and CS fixed

lorenzo added a commit that referenced this pull request Sep 30, 2015
Allow to migrate all migrations with one "mark_migrated" call
@lorenzo lorenzo merged commit 6f9f3da into cakephp:master Sep 30, 2015
@lorenzo
Copy link
Member

lorenzo commented Sep 30, 2015

👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants