Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

task rollback:all tries to delete table schema #295

Closed
steveoh opened this Issue Aug 23, 2012 · 17 comments

Comments

Projects
None yet
4 participants

steveoh commented Aug 23, 2012

I have a custom IVersionTableMetaData class that just renames the table and sets the schema that it runs under. When I try to roll back all migrations when the version table has no entries it tries to delete the version table, and then the schema which fails since that schema is used elsewhere and the whole transaction rolls back.

I had to add a dummy value to the table to avoid having it generate the schema drop statement. I can't find in the code where this happens so unfortunately I can't submit a pull request.

Actually it is right here. So why do you do this?

verbose error

Owner

tommarien commented Oct 7, 2012

I don't know what do suggest here, it depends if fluentmigrator creates the schema or not. Maybe adding an option OwnSchema ?

steveoh commented Oct 8, 2012

that seems good enough.

Contributor

daniellee commented Oct 9, 2012

I'm thinking that our current default (to remove the schema) is the wrong default. And then we can provide an option in IVersionTableMetaData to remove the schema. (See https://github.com/schambers/fluentmigrator/wiki/Create-custom-metadata-for-the-VersionInfo-table)

Owner

tommarien commented Oct 9, 2012

I don't share that oppinion, it think the normal use case is either you care about the schema, so you specify it or you don't care means not specifying it in your version meta data. Either way we choose, it should not affect the existing behaviour. Too me this means that we should delete by default if it was specified.

Just my two cents though :)

Contributor

daniellee commented Oct 9, 2012

I see it like this. If this is an existing database with an existing schema and I define my VersionInfo class to use that schema then I do not want it to delete (or try to delete) that schema. And this is more incorrect than leaving the schema alone after the final rollback.

Contributor

daniellee commented Oct 9, 2012

After thinking about this a bit more. It doesn't really matter what the default is as long as you can change it. I'll try and get that part done during the next day or two.

Owner

tommarien commented Oct 9, 2012

Thanks @daniellee

steveoh commented Oct 10, 2012

thanks guys! ✌️

Contributor

TheCloudlessSky commented Jan 11, 2013

What's the default with rails migrations?

Contributor

daniellee commented Jan 12, 2013

They don't really do schemas in rails migrations. They do not have anything like FluentMigrator's Create.Schema expression and I think that most Rails developers never create a new schema.

Owner

tommarien commented Jun 3, 2013

Was thinking again of this one, is a rollback:allnoschema sufficient ?

steveoh commented Jun 3, 2013

That name confuses me a bit. Does that mean it will rollback and delete the schema or ignore trying to delete the schema?

Owner

tommarien commented Jun 4, 2013

@steveoh was just thinking of a simple approach to this, allwithoutschemaremove or something

steveoh commented Jun 4, 2013

can I suggest that since the current CLI option is

rollback:all that it be some good names would be

rollback:allKeepSchema
rollback:allToSchema
rollback:all -keepSchema

I know names can be argued forever so I'll be happy with whatever you decide.

Owner

tommarien commented Jun 5, 2013

@steveoh I think we can do it very easy by adding a DeleteVersionSchema on IVersionMetadata, i'll try to whip up a pull for this in the coming days ;)

@tommarien tommarien was assigned Jun 5, 2013

Owner

tommarien commented Nov 22, 2013

@daniellee You didn't agree here on the approach i took ? ;)

@tommarien tommarien closed this in 11f6b25 Dec 29, 2013

@tommarien tommarien added a commit that referenced this issue Dec 29, 2013

@tommarien tommarien Merge pull Request #427 of tommarien/Issue295
* tommarien-Issue295:
  Renamed IVersionTableMetaDataEx to IVersionTableMetaDataExtended, just to reveal intend better
  Added IVersionMetaDataEx(tended) closes  #295 Choose the least breaking way of implementing this feature
9e28e22

steveoh commented Jan 17, 2014

👏 this is great. using the new version now.

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