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

Ability to perform atomic database migrations #3868

Open
patricknelson opened this issue Feb 9, 2015 · 11 comments
Open

Ability to perform atomic database migrations #3868

patricknelson opened this issue Feb 9, 2015 · 11 comments

Comments

@patricknelson
Copy link
Contributor

Currently the SilverStripe framework lacks the ability to easily perform custom migrations at an atomic level. That is, SilverStripe has a very intuitive interface for defining database structure in a declarative format (e.g. "setup these columns with this format and these relations") but currently there is no method that I know of which facilitates a smooth and controlled atomic transition of database schema and the data contained therein. I'm not aware of any techniques by which I can change the way data is handled by DataObjects so I can deploy my code and database changes without leaving behind code to represent vestigial column names or data formats (for example).

Just a few more specific examples:

  • Renaming Column: If a developer needs to simply rename a column, there's currently no clean method by which you can ensure that the new column will contain the existing data. Granted, the dev/build task will ensure that the new column is setup while leaving the old column in the database meaning the data isn't technically lost (which is fine), however I'm not aware of any clean techniques to move the data from the old column into the new column and then (optionally, now that I've safely copied the data) remove the old column from the DB schema so clean things up.
  • Changing Format: Say I decide to change the format of my data that the user enters into the CMS (and also in the database) by combining two redundant fields into one, like having FirstName and Lastname to just FullName, I cannot do so cleanly without leaving behind lots of legacy code in my data models. For example, I must now have 3 fields in the DataObject to ensure I still have access to old data (even though we know dev/build will not remove the fields). This is because we still must be able to call $person->FirstName in order to pre-populate the new FullName field, which means I must also create a getter method ->getFullname() which must call ->getField() to see if the current value is empty. That causes other issues, especially if the FullName can optionally be empty and the CMS user decides that they no longer wish the person (for whatever reason) to have an entry in that field, just as an example.

I noticed that there is some biolerplate code that introduces the concept in the framework here:
http://api.silverstripe.org/3.1/class-MigrationTask.html

I'd like to suggest that we be able to do the following (things I'd be happy to try implement on my own, with permission):

  • Build a task/script to generate migrations with file naming convention similar to Laravel. E.g. sake dev/migrate make NameOfMyMigration which would create a file in a default (or pre-defined) location using the following nomenclature: mysite/code/migrations/YYYY-MM-DD_TS_NameOfMyMigration.php where TS is either a full or partial UNIX timestamp to help ensure new migrations on the same day are executed in order (when alphabetized).
  • Newly created migration files (either automatically generated by helper above or manually created) would need extend a base SS_Migration object of some sort which would be responsible for defining abstract methods ->up() and ->down(). This would be similar to the MigrationTask in structure, but in principle the MigrationTask itself should be responsible for actually performing the task of migrating and would accept parameters on what to do (i.e. just migrate up, rollback, pretend or make a new migration for you) so intuitive it should not be a direct descendant of actual migrations. This suggests the task file responsibility should be changed (i.e. not extended but rather called directly as a dev build task) and that a new abstract class (e.g. SS_Migration) should be created which is responsible for defining base functionality/interface for actual migrations.
  • DataObject abstraction of some sort that keeps track of already run migrations. This will need some sort of namespacing and/or debate but I would propose SS_MigrationLog since SS_Migration would be needed above as a parent class for the actual migration files.
  • Integrate into the task/script the ability to run migrations up/down, e.g. sake dev/migrate (to migrate) or sake dev/migrate rollback to migrate down ("rollback changes").
  • Would be "nice to have" the ability to preview DB queries via a --pretend flag. I say "nice to have" since I'd assume that this would preferably capture (but not allow execution of) any modifying queries and list them out and indicate which migration file that is queued to run has called them. This will eventually be essential but wouldn't really be required immediately to incorporate baseline migration functionality.

Is there something already out there that does this (as a module) that I'm not finding or is this something we can implement? If so, what is the best approach? I'd be more than happy to contribute as much as I can. Any suggestions or recommendations would be appreciated!

@camfindlay
Copy link
Contributor

@patricknelson this looks like something that should be added over on our feature request space http://silverstripe.uservoice.com, this could also be the basis of an RFC if you are interested in proposing something (so far we've just started using this process of RFC for bigger pieces of work see #3792 for an idea of what they contain).

@patricknelson
Copy link
Contributor Author

Well, the RFC there seems pretty large and formal, so it's intimidating for me at first glance. On the flip side, I could submit a pretty informal suggestion and point it back here for formatting purposes. Do you think this might need to be in the form of an RFC to facilitate implementation? I suppose it depends on the overall internal impact, what SS wants and community feedback.

Anyway -- I went ahead and put something up here: http://silverstripe.uservoice.com/forums/251266-new-features/suggestions/7075780-ability-to-perform-atomic-database-migrations

I would definitely like to hear other people's thoughts (my own sort of "RFC") since I'd imagine this is another one of those "long time coming" features for those of us who have had experience with (read: "spoiled by") database migration tools in other systems. Honestly, for the type of functional and content heavy site I'm working on, it'll prove to be an extremely valuable tool.

@dhensby
Copy link
Contributor

dhensby commented Feb 10, 2015

I think it'll be a bit much to expect @patricknelson to draw up a formal RFC (unless you're planning on implementing what you suggest).

My view (and one I've heard as well) is that migrations are a bit of overkill given the automatic schema builder present in SS. However, there is support for it given a recent post to the mailing list

When I'm faced with this problem I'll usually write my own migration script that is run as part of the deployment process. It's easy to create a task and then just run it after the deployment process.

@patricknelson
Copy link
Contributor Author

The only problem (and the reason I don't think this is overkill) is due to one very important feature: The queuing of migrations and prevention of running migrations which have already executed. I'm just proposing a simple abstraction that would simplify and automate some of the redundant aspects of generating, de-duplicating and running migrations all in a single place.

The idea of using migrations is to handle that which dev/build does not (and cannot) because again it's working with declarative state. That is, the DataObject model just says what things should look like right now and dev/build will do what it takes to augment the current schema to make that happen. That's fine, but what happens when you have a site that's live in production which needs to have a new release deployed and you've had to restructure your code for clarity, bug fixes and etc but dev/build isn't smart enough to move/copy data from old columns into new ones? That's where migrations come in and the need to ensure there's a central mechanism for tracking what has and has not been run.

I like what Gregory has done and outlined in his message. As you can tell this is a common problem which, due to the lack of existing features, is possibly being re-implemented independently by developers who need it. Ideally we'd have a very good baseline (generic and extensible) abstraction which can handle the most common use cases and give power to the developer to do everything else they need.

As you can tell from the message, some common functionality (which is typical of migration systems) such as:

  • Ensuring migrations are always run in order (hence the YYYY-MM-DD nomenclature of the migration files, which I cover with slightly more detail using an additional time stamp) to allow multiple migrations in a day, which I may need to do frequently in a large complex project in other frameworks.
  • Keeping record of previous migrations and not running them multiple times.
  • Ability to migrate up as well roll back (down()).

Check out Laravel to see how they do it and get an idea of the context of what I'm referring to (just in case). That is where I've gotten the bulk of my migration experience. http://laravel.com/docs/4.2/migrations

@patricknelson
Copy link
Contributor Author

Also, I agree that migrations will be overkill for a large portion of the people who may adopt SS. However, implementation for me would be reasonably trivial considering the payoff would be quite high if I'm not the only person who ends up benefiting. Especially if you're in a situation like I am where you may need that extra bit of control on how the data is structured over time for complex content-heavy websites where requirements are shifting constantly (unfortunately real-world issues necessitate these continuous changes, sometimes).

Even if the system isn't changing frequently, it's always fantastic to know you've got a way to automate your deployment process consistently across the board.

@patricknelson
Copy link
Contributor Author

Funny -- I'm also just happening to notice this recently revived discussion here too: https://groups.google.com/forum/#!topic/silverstripe-dev/niQhqfK-h44

And: https://groups.google.com/forum/#!topic/silverstripe-dev/ebKhtcmgwRc

I can tell I'm not far off by indicating that this has probably been re-invented over and over again by other developers with the same need. People should definitely be pointed to the same place so we can tackle this centrally and coordinate our discussion and efforts. It seems people may not be dropping them into http://silverstripe.uservoice.com/ (at least not with the terms "migrate" or "migration").

@patricknelson
Copy link
Contributor Author

I posted a message in the discussion list but wanted to link here as well. I setup a module for this in case anyone finds it useful!

https://github.com/patricknelson/silverstripe-migrations/

@chillu
Copy link
Member

chillu commented May 8, 2017

@dhensby suggests moving to dbal, which would allow for using https://github.com/doctrine/migrations/

@bummzack
Copy link
Contributor

I've seen there's a MigrationTask in SS4, but somehow there's no way to specify the Direction param, or am I missing something?

@patricknelson
Copy link
Contributor Author

@bummzack That doesn't even really do anything, not sure why it's been kept since its mere existence is confusing (https://github.com/silverstripe/silverstripe-framework/blob/master/src/Dev/MigrationTask.php).

If you want atomic database migrations, I've got a module I built and have had in use for several years now (circa early 2015 or so) which has served me very well: https://github.com/patricknelson/silverstripe-migrations

Please give it a shot on SS4 and if you have any issues (likely as I've not tested it in SS4) please feel free to submit a PR!

@lekoala
Copy link
Contributor

lekoala commented May 3, 2018

just a quick note relative to the MigrationTask class : if someone can make it abstract so that it stops showing up in dev/tasks for no reasons, that would be great :-)

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

No branches or pull requests

8 participants