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

Auto migrate the version table #1

Closed
wants to merge 111 commits into from
Closed

Auto migrate the version table #1

wants to merge 111 commits into from

Conversation

goetas
Copy link
Owner

@goetas goetas commented Oct 3, 2019

This is a proposal on how to migrate the v2 versions table to v3 versions.

The thing that worries me here is that we do not ask the user if we should alter the table or not.

Does it make sense to have a separate command for it?

@goetas goetas mentioned this pull request Oct 3, 2019
5 tasks
@goetas
Copy link
Owner Author

goetas commented Oct 9, 2019

ping @alcaeus

@alcaeus
Copy link

alcaeus commented Oct 11, 2019

The thing that worries me here is that we do not ask the user if we should alter the table or not.

I'm not sure if a separate command on its own would be the solution.

For most commands, we can ask an interactive question if we see that there's a diff to the migrations table. If the users approves, we update their table and then continue the migration normally. If they don't want to upgrade, we abort the migration.

For commands that are run non-interactively, e.g. as part of a deployment, we obviously can't get any consent, so I believe the best course of action would be to tell people to run a separate command to update manually, then restart the migration process.

@OskarStark
Copy link

Does it make sense to have a separate command for it?

I would say yes 👍

@goetas
Copy link
Owner Author

goetas commented Oct 18, 2019

There are the following options:

  1. Do silently the conversion on the migrations_versions table
  • pro: simple and convenient, no extra actions are needed from the user
  • con: implicitly writing to the database (a write command will be issued on a simple migration:status command as example
  1. Have a separate migration command to update the migrations_versions, if another command is issued and the table is not up to date, return an error
  • pro: simple to implement
  • con: now we introduce a new step to be performed. Probably will annoy who uses automated processes in CI/CD
  1. Add a --allow-metadata-update parameter to all the commands. Apply option N.1 if the param is not specified. Return an error if the parameter is not specified and the table is not up to date. Still offer a command for manually migrating the table (meas offer N2 too)
  • pro: no unexpected behaviors that the user is not aware of (everything is explicit)
  • con: a bit more stuff to implement
  • mixed pro/cons: new --allow-metadata-update to be added on CI/CD pipelines

I'm personally for option n.3. Any other idea?

@alcaeus
Copy link

alcaeus commented Oct 18, 2019

I would agree that method 3 is the safest choice, as it will require people to acknowledge the change and decide whether they want to individually approve every version table migration or allow it to be updated automatically

@goetas goetas force-pushed the v30-auto-upgrade-table branch from c2211cc to c6a92b6 Compare October 22, 2019 14:48
@goetas
Copy link
Owner Author

goetas commented Oct 22, 2019

  • added checks in "read" commands that detect if the storage is not up to date
  • added command to update metadata storage
  • auto-migrating metadata storage when running migrations
  • added tests for related updates of schema and data

@goetas
Copy link
Owner Author

goetas commented Dec 1, 2019

Moving to doctrine#878

@goetas goetas closed this Dec 1, 2019
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.

3 participants