Container aware migrations #28

Merged
merged 2 commits into from Oct 18, 2012

Conversation

Projects
None yet
@frosas
Contributor

frosas commented Apr 20, 2012

This would allow much more powerful migrations as right now all you can do is SQL stuff.

Moved from https://github.com/symfony/DoctrineMigrationsBundle/pull/2

Container aware migrations
This would allow much more powerful migrations as right now all you can do is SQL stuff.

Moved from https://github.com/symfony/DoctrineMigrationsBundle/pull/2
@ryall

This comment has been minimized.

Show comment Hide comment
@ryall

ryall Jun 19, 2012

This would be most helpful

ryall commented Jun 19, 2012

This would be most helpful

@Burgov

This comment has been minimized.

Show comment Hide comment
@Burgov

Burgov Jun 27, 2012

big 👍

Burgov commented Jun 27, 2012

big 👍

@webignition

This comment has been minimized.

Show comment Hide comment
@webignition

webignition Jul 14, 2012

There may be an issue with how this patch calls $this->injectContainerToMigrations($container, $configuration->getMigrations());.

The call is being made from a static method and can result in a "Using $this when not in object context;" fatal error. It does for me, whether it does for all may be a matter of configuration.

Changing the call to self::injectContainerToMigrations and declaring injectContainerToMigrations to be static resolves this matter.

Would this matter affect others? If so, should the above changes be made, this pull request closed and a new pull request made?

There may be an issue with how this patch calls $this->injectContainerToMigrations($container, $configuration->getMigrations());.

The call is being made from a static method and can result in a "Using $this when not in object context;" fatal error. It does for me, whether it does for all may be a matter of configuration.

Changing the call to self::injectContainerToMigrations and declaring injectContainerToMigrations to be static resolves this matter.

Would this matter affect others? If so, should the above changes be made, this pull request closed and a new pull request made?

@frosas

This comment has been minimized.

Show comment Hide comment
@frosas

frosas Jul 14, 2012

Contributor

@webignition solved, thanks for noting this!

Contributor

frosas commented Jul 14, 2012

@webignition solved, thanks for noting this!

@qpleple

This comment has been minimized.

Show comment Hide comment
@qpleple

qpleple Jul 22, 2012

Would it be best practice to access the container in a Doctrine migration?

Say I have 3 fields to store a date day, month and year that I want to combine into a single field date. Using this feature, I could write a migration that would in method up():

  1. Add the field date
  2. Use the container to get the entity manager, then update the table filling the field date according to the values of day, month and year
  3. Remove the fields day, month and year

The other solution is to write 2 doctrine migration and a custom command, and run them in that order: migration that add the field date, custom command that fill that field and the second migration that remove the 3 fields. But then, if your migrations are in the middle of others, if you run the command
app/console doctrine:migrations:migrate
it will lose the information of the date (as the custom command has not been run).

qpleple commented Jul 22, 2012

Would it be best practice to access the container in a Doctrine migration?

Say I have 3 fields to store a date day, month and year that I want to combine into a single field date. Using this feature, I could write a migration that would in method up():

  1. Add the field date
  2. Use the container to get the entity manager, then update the table filling the field date according to the values of day, month and year
  3. Remove the fields day, month and year

The other solution is to write 2 doctrine migration and a custom command, and run them in that order: migration that add the field date, custom command that fill that field and the second migration that remove the 3 fields. But then, if your migrations are in the middle of others, if you run the command
app/console doctrine:migrations:migrate
it will lose the information of the date (as the custom command has not been run).

@frosas

This comment has been minimized.

Show comment Hide comment
@frosas

frosas Jul 22, 2012

Contributor

@qpleple I would stick with SQL in your example as all you need is already in the database and accessing the container doesn't give you much. This PR has in mind other cases like updating your database with data from external sources or from logic scattered all over your services.

Contributor

frosas commented Jul 22, 2012

@qpleple I would stick with SQL in your example as all you need is already in the database and accessing the container doesn't give you much. This PR has in mind other cases like updating your database with data from external sources or from logic scattered all over your services.

@qpleple

This comment has been minimized.

Show comment Hide comment
@qpleple

qpleple Jul 23, 2012

@frosas Yes, I know my toy example can be done only in SQL. I was asking about the idea of indeed having to use some logic of the application in the migration in order to chain migrations without worrying about running custom commands between them.

qpleple commented Jul 23, 2012

@frosas Yes, I know my toy example can be done only in SQL. I was asking about the idea of indeed having to use some logic of the application in the migration in order to chain migrations without worrying about running custom commands between them.

@frosas

This comment has been minimized.

Show comment Hide comment
@frosas

frosas Jul 23, 2012

Contributor

@qpleple oh yes, this is the main point, to completely automate the deploys :)

Contributor

frosas commented Jul 23, 2012

@qpleple oh yes, this is the main point, to completely automate the deploys :)

@webignition

This comment has been minimized.

Show comment Hide comment
@webignition

webignition Jul 23, 2012

@frosas What can container aware migrations achieve that Doctrine Fixtures don't?

@frosas What can container aware migrations achieve that Doctrine Fixtures don't?

@frosas

This comment has been minimized.

Show comment Hide comment
@frosas

frosas Jul 23, 2012

Contributor

@webignition fixtures can do the job to initialize the database but what about later?

Contributor

frosas commented Jul 23, 2012

@webignition fixtures can do the job to initialize the database but what about later?

@webignition

This comment has been minimized.

Show comment Hide comment
@webignition

webignition Jul 24, 2012

@frosas I agree entirely, I asked merely because this might be a question raised by the maintainers. I'm currently using a container-aware fork of DoctrineMigrationsBundle and use the postUp() and postDown() methods to inject essential data in ways that fixtures can't.

@frosas I agree entirely, I asked merely because this might be a question raised by the maintainers. I'm currently using a container-aware fork of DoctrineMigrationsBundle and use the postUp() and postDown() methods to inject essential data in ways that fixtures can't.

@hlecorche

This comment has been minimized.

Show comment Hide comment
@hlecorche

hlecorche Aug 10, 2012

@frosas I agree

@frosas I agree

@AsaAyers

This comment has been minimized.

Show comment Hide comment
@AsaAyers

AsaAyers Sep 4, 2012

I came looking for this because I have a legacy database where times are stored using the user's timezone, which is stored next to it in the database. Converting the existing date/time values can't be done with simple SQL.

I need to

  1. add a timestamp column
  2. loop over all rows converting all stored times to a single timezone placing the converted time in the new column
  3. drop old date/time column
  4. rename new column to replace the old name

Without access to the container i'm going to have to create multiple database migrations and remember that I have to run a custom command that will only be used once between steps 1 and 2.

I don't know much about DoctrineFixtures, but it doesn't appear to be what I need.

AsaAyers commented Sep 4, 2012

I came looking for this because I have a legacy database where times are stored using the user's timezone, which is stored next to it in the database. Converting the existing date/time values can't be done with simple SQL.

I need to

  1. add a timestamp column
  2. loop over all rows converting all stored times to a single timezone placing the converted time in the new column
  3. drop old date/time column
  4. rename new column to replace the old name

Without access to the container i'm going to have to create multiple database migrations and remember that I have to run a custom command that will only be used once between steps 1 and 2.

I don't know much about DoctrineFixtures, but it doesn't appear to be what I need.

@diegosainz

This comment has been minimized.

Show comment Hide comment
@diegosainz

diegosainz Sep 5, 2012

We have been using doctrine container aware migrations lately and is important to consider the following scenario:

  1. Make some changes to your entities and create migration-1 that uses SQL and entity manager migration (This is commit-1)
  2. Update your source code and add some new property to an entity used in migration-1 and create migration-2 that will use SQL to add the new column (This is commit-2)
  3. Run both migrations (with the code at the latest commit)
  4. An exception will be thrown because when migration-1 runs the metadata of the entity will already expect the column that wont be created until migration-2 runs.

The only solution I can think of is running each migration synchronized with the source code it was created. The other possibility is to run the entity manager migrations at the end - but maybe some SQL migrations expect the data that those entity-manager migrations would modify - or maybe save and load the entity metadata snapshot with each migration???

We have been using doctrine container aware migrations lately and is important to consider the following scenario:

  1. Make some changes to your entities and create migration-1 that uses SQL and entity manager migration (This is commit-1)
  2. Update your source code and add some new property to an entity used in migration-1 and create migration-2 that will use SQL to add the new column (This is commit-2)
  3. Run both migrations (with the code at the latest commit)
  4. An exception will be thrown because when migration-1 runs the metadata of the entity will already expect the column that wont be created until migration-2 runs.

The only solution I can think of is running each migration synchronized with the source code it was created. The other possibility is to run the entity manager migrations at the end - but maybe some SQL migrations expect the data that those entity-manager migrations would modify - or maybe save and load the entity metadata snapshot with each migration???

@AsaAyers

This comment has been minimized.

Show comment Hide comment
@AsaAyers

AsaAyers Sep 5, 2012

That's a good point. Sounds like using Entities in migrations is dangerous and cumbersome. I think the better solution might be to expose a database connection to run SQL (or DQL?) directly. If DQL is bound to how your entities are defined it may have the same problem.

I've chosen to use an external script that calls doctrine:migrations:migrate with one revision, then goes and runs other commands, then comes back to run doctrine:migrations:migrate to upgrade to the latest version.

AsaAyers commented Sep 5, 2012

That's a good point. Sounds like using Entities in migrations is dangerous and cumbersome. I think the better solution might be to expose a database connection to run SQL (or DQL?) directly. If DQL is bound to how your entities are defined it may have the same problem.

I've chosen to use an external script that calls doctrine:migrations:migrate with one revision, then goes and runs other commands, then comes back to run doctrine:migrations:migrate to upgrade to the latest version.

@frosas

This comment has been minimized.

Show comment Hide comment
@frosas

frosas Sep 5, 2012

Contributor

@diegosainz yes, this is a very good reason to stay in the lower layers in the migrations.

What I've done in these cases is to create major versions (i.e. branches) of the code. When the changes in the code broke the migrations and fixing them was not worth the hassle I created a new version.

Contributor

frosas commented Sep 5, 2012

@diegosainz yes, this is a very good reason to stay in the lower layers in the migrations.

What I've done in these cases is to create major versions (i.e. branches) of the code. When the changes in the code broke the migrations and fixing them was not worth the hassle I created a new version.

@ludekstepan

This comment has been minimized.

Show comment Hide comment
@ludekstepan

ludekstepan Sep 20, 2012

Please look at #27 for working simple alternative approach.

Please look at #27 for working simple alternative approach.

beberlei added a commit that referenced this pull request Oct 18, 2012

Merge pull request #28 from frosas/patch-1
Container aware migrations

@beberlei beberlei merged commit e494ace into doctrine:master Oct 18, 2012

@frosas frosas deleted the frosas:patch-1 branch Nov 12, 2014

@webdevilopers

This comment has been minimized.

Show comment Hide comment
@webdevilopers

webdevilopers Nov 13, 2014

👍

👍

@webdevilopers webdevilopers referenced this pull request in doctrine/migrations-documentation Nov 13, 2014

Open

Add docs for pre*, post* and *if methods #9

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