[RFC] Migrations

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
User avatar
callumacrae
Former Team Member
Posts: 1046
Joined: Tue Apr 27, 2010 9:37 am
Location: England
Contact:

Re: [RFC] Migrations

Post by callumacrae »

brunoais wrote:
Oleg wrote:
As for timestamp.php, how will this not work for more than 1 migration?
Which of the following files contain changes for pruning users?

1354877513.php
1354711242.php
1354308639.php
1354108598.php
1354748196.php
1354522012.php
1354375160.php
1354826681.php
1354836491.php
1354842171.php
1354091588.php
1354517488.php
1354325118.php
1354612444.php
1354208440.php
1354360076.php
1354479350.php
1354836279.php
1354581861.php
1354306151.php
1354314469.php
1354843508.php
1354114481.php
1354329646.php
1354675411.php
1354687288.php
1354570385.php
1354214827.php
1354868628.php
1354598616.php
1354126757.php
1354428804.php
1354499330.php
1354441865.php
1354815033.php
1354103679.php
1354857156.php
1354861770.php
1354916617.php
1354490787.php
I think that can be solved with something like...
[meaningfulNameOfMigration]_[TIMESTAMP].php
That's what he's arguing for. Timestamp first though, please!
Made by developers, for developers!
My blog

User avatar
DavidIQ
Customisations Team Leader
Customisations Team Leader
Posts: 1904
Joined: Thu Mar 02, 2006 4:29 pm
Location: Earth
Contact:

Re: [RFC] Migrations

Post by DavidIQ »

I think the timestamp should go second. Otherwise in the example provided it would be difficult to find the pruning users changes file(s).
Image

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: [RFC] Migrations

Post by imkingdavid »

brunoais wrote:I think that can be solved with something like...
[meaningfulNameOfMigration]_[TIMESTAMP].php
DavidIQ wrote:I think the timestamp should go second. Otherwise in the example provided it would be difficult to find the pruning users changes file(s).
Timestamp should be first so that it correctly sorts files by the time rather than by an arbitrary name given to describe the timestamp. As long as you have a decent search feature in your file manager, finding filenames containing "prune" or maybe even "prun*" (to match prune and pruning; if you can do regex as well) should not be difficult.
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
callumacrae
Former Team Member
Posts: 1046
Joined: Tue Apr 27, 2010 9:37 am
Location: England
Contact:

Re: [RFC] Migrations

Post by callumacrae »

ls | grep "prune"
Made by developers, for developers!
My blog

User avatar
DavidIQ
Customisations Team Leader
Customisations Team Leader
Posts: 1904
Joined: Thu Mar 02, 2006 4:29 pm
Location: Earth
Contact:

Re: [RFC] Migrations

Post by DavidIQ »

I don't think it's that big of a deal making the timestamp first if it's going to make things easier (I don't really see how) and I'm not going to argue against it. Users can just download all of their migration files locally and search as either of you have specified although downloading 20+ migrations to just look at 1 file seems like a waste of bandwidth and time to me. :| Expecting users to have shell access to their sites is also not realistic.
Image

User avatar
MichaelC
Development Team
Development Team
Posts: 889
Joined: Thu Jan 28, 2010 6:29 pm

Re: [RFC] Migrations

Post by MichaelC »

Migrations is write only code? Migrations, once made, should never be changed. If you need to make a change you make a new migration.

Example that has the behavior I've described is Doctrine Migrations (except they allow timestamps or versions or whatever you choose. The symfony doctrine migrations bundle uses timestamps).

The only time migrations will ever be read it during code review, which can be solved by creating a basic diff tool (can be used to show the resulting changes from one migration to another with all in between), as is done with doctrine. What is a use case of why a migration for pruning users would need looking at? Anyway, it is unlikely there would be 1 migration for pruning users, but likely there would be many as it goes through development. Migrations should never be changed once published or applied to any database.

Generation is not a 'reduction to functionality' as if you choose you could still do it manually, it just offers and alternative option. I've been using an auto-generator for migrations for db schema a lot and have never had any errors so please show how it would be 'error prone'.

Breaking - Human error whilst creating a migration.

I understand the timestamp on it's own cannot happen, but I still don't think that a descriptor should differ. Version or Migration would do fine.

See https://github.com/phpbb/phpBBStatusSit ... Migrations for an example of extremely efficient migrations used on phpBB sub-projects already in the way I've described. None of the problems you have mentioned have come near us. The majority of symfony applications use the same thing (those that use doctrine & a migrator) and I have not seen many issues at all compared to the massive volume of use (Install numbers here and here). I use doctrine migrations as my example throughout this post as is it the one I am most familiar with and one of the most used (due to the large market share of doctrine) but I'm sure there are others that work in a similar way.
Formerly known as Unknown Bliss
psoTFX wrote: I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"
No unsolicited PMs please except for quotes.

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC] Migrations

Post by Oleg »

See https://github.com/phpbb/phpBBStatusSit ... Migrations for an example of extremely efficient migrations used on phpBB sub-projects already in the way I've described.
I don't doubt that this works for you, however https://github.com/phpbb/phpBBStatusSit ... 841bbcad7f will not pass phpbb code review on commit message/commit size alone.
MichaelC wrote:Migrations is write only code?
Not when they are reviewed before merge and read later.

Write-only code is what you have in https://github.com/phpbb/phpBBStatusSit ... Migrations. Nobody will go and read those files. I wouldn't be surprised if nobody ever read them considering they were autogenerated.
Example that has the behavior I've described is Doctrine Migrations (except they allow timestamps or versions or whatever you choose. The symfony doctrine migrations bundle uses timestamps).
Yep. Thanks for pointing this out.
The only time migrations will ever be read it during code review, which can be solved by creating a basic diff tool
A "basic diff tool" is only useful if github runs it before rendering the diff. Code review is performed by reading diffs in github.
Generation is not a 'reduction to functionality' as if you choose you could still do it manually, it just offers and alternative option.
Generation instead of hand-writing migrations is a reduction in functionality. Generation in addition to hand-writing migrations is unnecessary. It's a nice feature but I would like to ship 3.1 sooner rather than later.
I've been using an auto-generator for migrations for db schema a lot and have never had any errors so please show how it would be 'error prone'.
https://github.com/phpbb/phpBBStatusSit ... Migrations use mysql syntax, therefore are a non-starter for phpbb as they are.

Does it rename foreign keys when it renames columns? (postgres)

Does it detect trigger modifications? (any db with triggers)
I understand the timestamp on it's own cannot happen, but I still don't think that a descriptor should differ. Version or Migration would do fine.
Thinking about this again, migrator should enforce that there are no two migrations with the same name across the entirety of possible locations (core+extensions). Essentially detection for accidental copy pastes.

Overall I still have the opinion that name is mandatory. We may allow it to be omitted for extensions if they so choose but all code merged into the core should have names on migrations.

As far as order of name and timestamp, from my experience chronology of migrations (timestamp) is more important than description of a particular migration (name). This is compounded by the fact that migration names are arbitrary and are not subject to any particular convention. For an implementation that sequences migrations by time only and does not allow for cross-migration dependencies as originally intended in this topic, migration files are ordered by time because that is the order they are applied in which is most important.

User avatar
MichaelC
Development Team
Development Team
Posts: 889
Joined: Thu Jan 28, 2010 6:29 pm

Re: [RFC] Migrations

Post by MichaelC »

I'm not going to quote you because otherwise we'll end up with massive posts but I'll number them to reply to each of your points.

1) How are commit messages relevant? I don't quite understand why you are bringing that into this. It isn't relevant, nor is this the place for discussion about website sub-project commit messages.
2) See 4 about the diff tool.
3) -
4) Well, it doesn't take much effort to run this diff tool and post the results in the PR? If it is then we could abuse travis or create a tool (could be done in a few lines of php) to run it for us. Anyway, this is how migrations would work no matter what (multiple migrations per PR). The diff tool is something to help you with code reviews. ;) If you'd rather read them and try and combine them in your head that is fine, but I know the majority of people who review PRs would prefer to be able to see the overall changes combined rather than doing that in their heads.
5) Perhaps we could add it in a later release. I wouldn't object to this.
6) Yes, because the migration would check the ORM (bear in mind that that example is using doctrine with ORM) entities, see the changes and using some clever logic work out what was changed. Any keys would be specified as annotations to that column in the entity. So the answer for that example is yes because of the usage of the ORM. But likely not in phpBB. As I said, generations wouldn't always be best in every situation, it would just be nice to have that available for basic usage when it can be used (adding/removing tables, columns etc.).
7) I didn't think about that. I suppose it is likely that at some point a phpBB core and extension entity would be generated at the same moment so +1 to having a descriptor. But bear in mind this doesn't help the 'which is for purging posts' senario as there could be 10 for the purging posts PR so the diff tool would come in helpful.

Something I thought of whilst writing this:
There needs to be a script to add a migration to the migrated migrations table without actually migrating it (as it would have likely already been applied manually whilst the developer was developing), however this means there could be issues where migrations are not fully tested; which would be less likely to happen with generations.
Formerly known as Unknown Bliss
psoTFX wrote: I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"
No unsolicited PMs please except for quotes.

User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: [RFC] Migrations

Post by nickvergessen »

MichaelC wrote:There needs to be a script to add a migration to the migrated migrations table without actually migrating it (as it would have likely already been applied manually whilst the developer was developing), however this means there could be issues where migrations are not fully tested; which would be less likely to happen with generations.
Something like this is already required, so that Mods/Extensions can be upgraded to from 3.0 to 3.1 otherwise you'd require the user to remove all stuff and for Mods like the gallery, blog and some more with a lot of content this is a no-go really.

I'd also vote for a +1 on useful name before timestamp, just for the simple reason that migrations that are related together are close together, (because they have the same prefix), rather then grouping up migrations by their time. This is also complete useless in cases like the user-timezone thing we had, where the branch was over 1 year old.
Member of the Development-TeamNo Support via PM

User avatar
EXreaction
Registered User
Posts: 1555
Joined: Sat Sep 10, 2005 2:15 am

Re: [RFC] Migrations

Post by EXreaction »

Just to give an update, I now seem to have migrations working at a basic level. Tests pass and I seem to be able to update from 3.0.0 to 3.1.0-dev with migrations (the database is nearly identical to one with database_update handling the update from 3.0.0 to 3.1.0-dev).

Now I think all we really need is some way to do roll-backs where possible in case an error occurs and migrations might be ready.

Post Reply