Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[RFC] Migrations #145

Closed
havvg opened this Issue · 32 comments

9 participants

@havvg

A main issue I see with the current implementation of the migration: they are streamlined, not allowing branching.

Each migration should be tracked separately, so one could branch the source add migrations to it, merge it back and run all migrations. This should also work when merging more than two branches!

The migration table should only insert every migration that has been done (up) and remove its entry if reverted (down).
In addition, it could be nice to know, when a certain migration has been done by adding a created_at column to the table.

@fzaninotto

Do you know an example implementation for such migrations in another ORM?

@willdurand
Owner

ping @themouette , I know you have ideas on this topic.

@willdurand
Owner

So, I stumbled upon Phinx this morning, and for a few months I thought about migrations into Propel. IMO we are bad at providing a decent migration tool, and Propel is not about migrations. It is an ORM, not a migration tool. IMO (again), we should use a third party tool/lib to handle migrations for us. In PHP there are a few libs I guess, and out of PHP's scope, there are plenty tools.

So, WDYT? Are you aware of libs that could fit our needs? Would you agree on removing the migration part from Propel, and instead relying on a third-party lib that we would integrate?

@willdurand
Owner

poke @propelorm

@staabm
Collaborator

whatever we decide, I think migrations should not block the intial propel2 release.. we could add it in a subsequent release.

@willdurand
Owner

Dropping the migration stuff before the first non-alpha/beta/whatever release would be nice though.

@tacman
@jaugustin
Collaborator

@willdurand I am -1 on this propel migrations do the job and knows well Propel's schema.xml.

I think we shouldn't drop migration for a 3rd party lib if the external lib can't do the same job.

@marcj
Owner

I'm -1 too, because Phinx supports only MySQL and PostgreSQL and Propel has already a powerful migration part.
I think in the direct opposit direction: We should implement in the migration part whatever is necessary, because we've already invested much time&know-how into it. Furthermore, we should highlight it more on the website since it's pretty stable and we support SQLite (which I guess almost no php migration tool does)

@havvg, what do you mean with not allowing branching? Was means branching in migration tools?

@havvg

@marcj What I mean is migrations within feature branches of your version control system. Currently it's not possible to branch migrations with Propel.

If you got a projects currently deployed version with migration version M, and you create e.g. two feature branches in your VCS, when merging the second branch into the master branch (deployed version), those developers with the feature branch containing the highest numeric M+x migration will not receive the M+y (y<x) migration from the other feature branch.

However I'm with you, that's not Propel issue, and one should reach out for a 3rd party migration tool.

@marcj
Owner

@havvg, I've never understood the up/down migration stuff completely, but why not just version control your schema.xml? Propel is able to migrate then all changes to your database and it's not necessary to have the full history of all changes.

@havvg

Well, because we are utilizing postUp to e.g. initialize required data. That's why we have the schema and the migrations under version control.

@willdurand
Owner

propel migrations do the job

No they don't. There are a few issues, and they are simplistic.

Phinx is not good "as is" as @marcj said, but we could contribute to support more databases I'd say.

@willdurand
Owner

@havvg would Phinx cover your use case?

@havvg

Yes, it does.

@robmorgan

Hey Guys, I'm the lead developer of Phinx. Is the reason you wouldn't use Phinx due to the lack of DB adapter support?

@willdurand
Owner

I think that we could write a PropelAdapter that would take a Propel adapter as argument.

I see that there is no diff support in Phinx but it is probably too specific.

We also need to think about how to map your commands to the Propel command line.

@jaugustin
Collaborator

@willdurand Phinx could be a great tools, but It will be annoying if you have to describe twice your database in schema.xml and in Phinx migrations.

@robmorgan would it be possible to extends your lib so that generated migration could be prefiled with info from schema.xml ?

@willdurand
Owner

I will try to play with Phinx and Propel.

@robmorgan

@jaugustin Yes I agree it would be annoying. We could try extend Phinx to work better with Propel, but I'd be against anything that is very Propel specific.

@jaugustin
Collaborator

@robmorgan I don't mean putting Propel specific code in Phinx lib, but have some Propel Command that use Phinx to prepare migration file with data from Propel's schema.xml

@robmorgan

@jaugustin ok great we're on the same page.

@marcj
Owner

The biggest thing I see is the lack of time. It would increase the delay of the Propel2 release dramatically and makes just no sense.

Why should we integrate a 3rd party lib that can't do the same job as we today? Ok, we get +1 feature for havvg's case but -3 other features are missing then - we need a big amount of time to implement this stuff into Phinx.
If someone will do it, I'd give +1. But I've already invested much time to fix some migration stuff and to integrate PostgreSQL and SQLite into the testsuite with full migration support. We have now a pretty stable/tested migration part - I won't do the same again for Phinx without a really good reason.

@marcj
Owner

@havvg, just to be clear here: If we change the output of the migration files from plain SQL to something like in Phinx $tableXy->addColumn('bla') etc. then your case is covered and it works then? I ask, because that wouldn't be much work as we already have all model classes (table, column, index, etc.).

@willdurand
Owner

The more I think about that RFC, the more I don't think it makes sense to rely on a third-party lib as:

  1. we have a strong Model layer in Propel2;
  2. we handle diffs;
  3. it has issues but 1. & 2. are better than most of the existing third-party libs.
@jaugustin
Collaborator

@willdurand Maybe the only missing thing is to handle parallel migrations we can do that by tracking all migrations that have been executed and not only the last one.
And maybe change the migration file format from sql to model addition/remove like phinx and only convert to sql when running

@havvg

@marcj as @jaugustin mentioned, the missing piece is the individual tracking of the migrations already executed. I like the current migrations, it's only this one missing piece :-)

@marcj
Owner

Ok guys, how can we fix this now? Is someone here who wants to pimp up the current migration to support havvg's case?

@mpscholten
Collaborator

It seems @javer already solved this for propel1, see propelorm/Propel#777. Maybe we can reuse his code for propel2?

@jaugustin
Collaborator

@marcj I wanted to work on this because I have some ideas to improve current situation and solves @havvg case, but I didn't find time to do it yet :(

Maybe @javer solution is a good start to fix @havvg use case.

@mpscholten
Collaborator

Looks like this issue can be closed because #551 added the missing functionality.

@marcj marcj closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.