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

Migrations: Make it easier to revert last migration when removing it #1972

Closed
maumar opened this issue Apr 1, 2015 · 14 comments
Closed

Migrations: Make it easier to revert last migration when removing it #1972

maumar opened this issue Apr 1, 2015 · 14 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Apr 1, 2015

When trying to remove already applied migration we get a message to unapply it first because it's already in the database. However unapplying first migration is not very intuitive, one has to type "dnx . ef migration apply 0" (since we don't have named migration for empty database)

I would much rather have some sort of -force option on ef migration remove that would unapply last migration and then remove it.

@rowanmiller
Copy link
Contributor

Yeah I think a general purpose flag (+ a prompt if it is applied and the flag isn't passed) to allow easily reverting the migration during removal would be good.

@rowanmiller rowanmiller changed the title Migrations :: we should make it easier to undo first migration Migrations :: we should make it easier to undo last migration Apr 20, 2015
@rowanmiller rowanmiller added this to the 7.0.0 milestone Apr 20, 2015
@guardrex
Copy link

guardrex commented Jun 3, 2015

Just ran into this as a new user of EF: It was not intuitive to apply the previous migration in order to unapply the current one in order to be able to remove the current migration.

It seems like if the normal EF sequence is to "add" then "apply" a migration, we should have commands for "unapply" and "remove" that work in the exact reverse. If not, then it might be a good idea that where the exception states:

System.InvalidOperationException: The migration '<migration_id>' has already been applied to 
the database. Unapply it and try again. If the migration has been applied to other databases, 
consider reverting its changes using a new migration.

it would be more helpful to add the phrase "by applying the previous migration," which would have made it very clear to me what I needed to do:

System.InvalidOperationException: The migration '<migration_id>' has already been applied to 
the database. Unapply it by applying the previous migration and try again. If the migration has 
been applied to other databases, consider reverting its changes using a new migration.

@guardrex
Copy link

guardrex commented Jun 3, 2015

... and "unapply" seems kind of hokey. The antonyms for "apply" are: cease, halt, ignore, mismanage, misuse, and neglect. Maybe "ignore" is the closest in meaning to "unapply" in this context. One would "add"-"apply" then "ignore"-"remove".

@bricelam
Copy link
Contributor

bricelam commented Jun 3, 2015

Revert is another word we typically use to mean the opposite of apply.

@bricelam
Copy link
Contributor

bricelam commented Jun 3, 2015

Splitting it into two commands--Apply-Migration and Revert-Migration--is a good idea. We could also rename the Up and Down methods to fully align.

@guardrex
Copy link

guardrex commented Jun 3, 2015

Ah, yes, "revert" is much better.

btw-- #2153 updates for EF migrations worked perfectly. Excellent technology! Next, I'm lick'in my chops for Azure Table Storage support when that arrives.

@rowanmiller
Copy link
Contributor

@bricelam love the command/method naming idea... has a nice symmetry to it 😄

@brent-russell
Copy link

I just ran into this trying my first "revert" test. @guardrex 's comments are spot on. I eventually figured it out via a "what if I apply the previous migration?" tangent, but an explicit revert command would be welcome.

FWIW, first I tried "remove" and got the "already applied" error message. Then I ran --help on both "remove" and "apply" to see if there were any options/flags that would either force "remove" to also revert, or reverse the direction of the apply command. I had a brief thought that maybe revert wasn't implemented, and then tried applying the previous migration by name to see what it would do. (I'm not the same new user GuardRex encountered.)

A "revert" command would also be beneficial because it would enable reverting the last applied migration without having to specify the previous migration name.

I don't think renaming the Up and Down methods is necessary. Those names are pretty intuitive. It wouldn't bother me if they were though. It's such a small change. Up and Down is the terminology used by FluentMigrator. So I guess it might be considered an "industry standard".

Thanks to everyone on this issue!

@bricelam
Copy link
Contributor

bricelam commented Jul 9, 2015

We talked a bit more about this on the team recently, and our current thoughts were to rename Apply-Migration to something like Migrate-Database. That way it's clearer that it can both apply and revert migrations. The help should read something like "Applies and reverts migrations in the database."

We came to the same conclusions on the Up and Down methods. They're pretty canonical across migration technologies, so keeping them is probably best.

We should add a --force switch to ef migration remove and update the error message to mention it.

@guardrex
Copy link

guardrex commented Jul 9, 2015

Cool, and I hope you'll consider modifying the exception a bit ... something like,

System.InvalidOperationException: The migration '<migration_id>' has already been applied to
the database. Unapply it to the previous migration by using the Migrate-Database command and
try again. If the migration has been applied to other databases, consider reverting its changes
using a new migration.

... we LOB devs aren't too smart (as I prove over and over here)! 😄

@natemcmaster
Copy link
Contributor

Apply-Migration 0 unapplies in the wrong order. It starts with the oldest migration. The first should be last and the last should be first.

@bricelam
Copy link
Contributor

@natemcmaster Would you mind filing a new issue? It's on my TODO list to investigate and write some Migrations end-to-end tests, but having an issue to track it would also be good.

@natemcmaster
Copy link
Contributor

@bricelam Here: #2695

@rowanmiller rowanmiller changed the title Migrations :: we should make it easier to undo last migration Migrations: Make it easier to un-apply last migration when removing it Dec 3, 2015
@rowanmiller rowanmiller removed the pri1 label Dec 8, 2015
@rowanmiller rowanmiller modified the milestones: Backlog, 7.0.0 Dec 8, 2015
@bricelam bricelam added the tools label Jan 13, 2017
@bricelam bricelam removed the tools label Apr 26, 2017
@bricelam
Copy link
Contributor

Note, we used --force to skip checking if the migration is applied. We should use something like --revert to mean just revert before removing it.

@bricelam bricelam changed the title Migrations: Make it easier to un-apply last migration when removing it Migrations: Make it easier to revert last migration when removing it Nov 27, 2017
@smitpatel smitpatel self-assigned this Nov 30, 2017
smitpatel added a commit that referenced this issue Nov 30, 2017
Resolves #1972

Also updated T4 template for string resources generation. It had some weird issues.

Did manual verification with 1 or more migrations.
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Nov 30, 2017
smitpatel added a commit that referenced this issue Nov 30, 2017
Resolves #1972

Also updated T4 template for string resources generation. It had some weird issues.

Did manual verification with 1 or more migrations.
smitpatel added a commit that referenced this issue Nov 30, 2017
Resolves #1972

Also updated T4 template for string resources generation. It had some weird issues.

Did manual verification with 1 or more migrations using PMC & dotnet ef both.
@smitpatel smitpatel modified the milestones: Backlog, 2.1.0 Nov 30, 2017
smitpatel added a commit that referenced this issue Nov 30, 2017
Resolves #1972

Also updated T4 template for string resources generation. It had some weird issues.

Did manual verification with 1 or more migrations using PMC & dotnet ef both.
smitpatel added a commit that referenced this issue Nov 30, 2017
Resolves #1972

Also updated T4 template for string resources generation. It had some weird issues.

Did manual verification with 1 or more migrations using PMC & dotnet ef both.
@ajcvickers ajcvickers modified the milestones: 2.1.0-preview1, 2.1.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

8 participants