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

Added parallel migrations #551

Merged
merged 1 commit into from
Mar 18, 2014
Merged

Conversation

mpscholten
Copy link
Member

This is a port of propelorm/Propel#777 to Propel2, without the backwards compatibility. This feature is request in #145. Basically it's just tracking of all executed migrations instead of the latest one.

Only problem is that I'm unable to write tests. I need to call $migrationManager->setConnections(..) in the tests but I'm not sure where to get the connection settings from. If someone could help me with this, I'll try to write some tests.

I would really like to see this feature in Propel2 because it's really hard to use propel migrations if you have multiple feature branches.

@willdurand
Copy link
Contributor

👍

@staabm
Copy link
Member

staabm commented Feb 25, 2014

please correct me if I am wrong: this PR removes the "old" way of doing migrations and does everything in parallel..?

propelorm/Propel#777 added the parallel part optionally (opt-in like approach)?

@willdurand
Copy link
Contributor

Yep, because you are no more tied to the migrations' timestamps anymore, if I understand correctly.

@mpscholten
Copy link
Member Author

added the parallel part optionally (opt-in like approach)?

Yes I dropped this because I think there's no benefit of keeping 2 ways of doing migrations.

@marcj
Copy link
Member

marcj commented Mar 3, 2014

Can you also write some tests for that?

@mpscholten
Copy link
Member Author

I need to call $migrationManager->setConnections(..) in the tests but I'm not sure where to get the connection settings from. If someone could help me with this, I'll try to write some tests.

I tried already :-) but I don't know how to set up the MigrationManager object in my tests properly. I need to call setConnections after creating the new object, but I don't know where to get the connection informations in the test case.

@marcj
Copy link
Member

marcj commented Mar 3, 2014

@mpscholten
Copy link
Member Author

But where do I get the generator config? $generatorConfig->getBuildConnections($input->getOption('input-dir'));

@marcj
Copy link
Member

marcj commented Mar 3, 2014

$generatorConfig = new GeneratorConfig();

:-)

@mpscholten
Copy link
Member Author

Thanks mate :) but $generatorConfig->getBuildConnections() is an empty array now

@marcj
Copy link
Member

marcj commented Mar 3, 2014

Yes, because you have to pass a directory of a fixture you want to use.

$generatorConfig->getBuildConnections(__DIR__. '/../../path/to/tests/Fixtures/migration/');

@mpscholten
Copy link
Member Author

Thanks, tests are ready now. Time for review

@gharlan
Copy link
Contributor

gharlan commented Mar 4, 2014

Hmm, all SQLite builds failed..

@mpscholten
Copy link
Member Author

Fixed. Maybe later we can port the tests back to propel1 so we can merge propelorm/Propel#777

@mpscholten
Copy link
Member Author

Any updates here? :-)

@willdurand
Copy link
Contributor

LGTM @marcj

@willdurand willdurand added this to the alpha-3 milestone Mar 16, 2014
@jaugustin
Copy link
Member

it's ok for me to,
@willdurand could you merge this PR ?
I am working on a new migration command and some improvement and it would be better if I could use this ;)

@willdurand
Copy link
Contributor

it needs to be rebased.

@mpscholten
Copy link
Member Author

Rebase done

@marcj
Copy link
Member

marcj commented Mar 18, 2014

Squash please :)

Fixed missing method

Added MigrationManager tests and fixed a bug in MigrationManager

Fix for Only variables should be passed by reference

Fixed migrationTableExists on sqlite
@mpscholten
Copy link
Member Author

Squashed :shipit:

marcj pushed a commit that referenced this pull request Mar 18, 2014
@marcj marcj merged commit 24bc29e into propelorm:master Mar 18, 2014
@marcj
Copy link
Member

marcj commented Mar 18, 2014

Good job 👯

@jaugustin jaugustin mentioned this pull request Mar 20, 2014
@mpscholten mpscholten mentioned this pull request Apr 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants