-
-
Save imkingdavid/cc23ddbf247574397b61 to your computer and use it in GitHub Desktop.
IRC Log discussing migration roll backs for dropped tables. - end result: we probably shouldn't worry about it unless it actually becomes a problem.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[13:37:46] <nickvergessen> is migrations done now | |
[13:41:26] <imkingdavid> nickvergessen: the backend part has been merged but there are two more related PRs | |
[13:41:34] <nickvergessen> yeah | |
[13:41:36] <nickvergessen> just found that | |
[13:41:55] <imkingdavid> i'm working on testing the feature/migrations-extensions pr | |
[13:42:04] <imkingdavid> has some stuff to be changed in it | |
[13:46:56] --> spyro (~shentino@gentoo/contributor/shentino) has joined #phpBB-dev | |
[13:49:38] <-> erikfrerejean is now known as erik|away | |
[13:50:40] <nickvergessen> hmm | |
[13:51:55] <nickvergessen> https://area51.phpbb.com/phpBB/viewtopic.php?p=250441#p250441 | |
[13:52:02] <nickvergessen> was not addressed in any way | |
[13:54:29] <imkingdavid> leave a note for nathan or remind him in the topic | |
[14:01:52] <naderman> imkingdavid: :) | |
[14:02:34] <imkingdavid> naderman: :) | |
[14:02:46] <imkingdavid> one small step for a man, one large step for a bertie | |
[14:02:57] <naderman> heh | |
[14:03:28] --> Raimon (~Adium@phpbb/manager/Raimon) has joined #phpBB-dev | |
[14:03:28] *** Mode #phpBB-dev +o Raimon by ChanServ | |
[14:04:45] <nickvergessen> one big fail for bertie | |
[14:04:51] <nickvergessen> this is serious and needs fixing | |
[14:08:59] <imkingdavid> nickvergessen: so the issue, as i understand it, is that one extension potentially drops a common table on which another extension has created a column, therefore potentially breaking the other extension? I'm not sure what can be done to prevent that | |
[14:09:23] <imkingdavid> should we just make it so no default phpbb tables can be dropped by extensions? | |
[14:09:31] <nickvergessen> remove potential from teh sentences | |
[14:09:47] <nickvergessen> no | |
[14:09:53] <nickvergessen> it needs general fixing | |
[14:10:33] <nickvergessen> and basically its simple | |
[14:10:37] <imkingdavid> well i'm assuming the mod team will at least glance at an extension's migrations before approving and if an extension tries to drop a default table that should raise red flags instantly | |
[14:10:50] <nickvergessen> you just need to log the current structure befor you start to do something | |
[14:11:15] <imkingdavid> so do an export of the table you're going to drop, and save that somewhere in the migrations table? | |
[14:12:16] <nickvergessen> yeah something like that | |
[14:12:39] <imkingdavid> let's say an extension drops the config table. now the board is completely borked. how does one solve this issue, just because they saved the schema in the database? | |
[14:12:52] <imkingdavid> they can hunt through the database in phpmyadmin to find the table schema and import it | |
[14:12:56] <imkingdavid> but it still doesn't have any data | |
[14:13:06] <imkingdavid> so it still doesn't matter that the schema was saved | |
[14:13:07] <nickvergessen> we dont care about that | |
[14:13:12] <imkingdavid> what do we care about then? | |
[14:13:18] <imkingdavid> i'm not understanding what you're trying to prevent | |
[14:13:19] <nickvergessen> if an extension creates an additional column on a table | |
[14:13:25] <nickvergessen> and another one drops it | |
[14:13:35] <nickvergessen> reverting the drop (by uninstalling the extension) | |
[14:13:50] <nickvergessen> will not restore the additional columns of the other extension | |
[14:15:59] <imkingdavid> I don't know why were so concerned about keeping the structure intact, when dropping a table is going to clear all data as well. both cases will required a restore from a recent backup | |
[14:16:14] <nickvergessen> revert should be a revert | |
[14:16:19] <nickvergessen> and not a reset to default | |
[14:19:03] <imkingdavid> I'm still not sure we should allow default tables to be dropped at all by extensions. I can think of 0 use cases for this, and allowing it only presents problems like this and others | |
[14:19:11] <imkingdavid> naderman: do you have any thoughts on this issue ^ | |
[14:20:04] <nickvergessen> imkingdavid: this also applies to tables of extension | |
[14:20:05] <nickvergessen> s | |
[14:20:12] <naderman> it's an extensions job to work correctly | |
[14:20:23] <naderman> if an extension drops a table in a migration it better recreate it on rollback | |
[14:20:29] <naderman> we can't magically fix broken extensions | |
[14:20:35] <naderman> a broken mod could drop a table today too | |
[14:20:35] <nickvergessen> which it currently doesnt | |
[14:20:42] <naderman> nickvergessen: huh? | |
[14:20:49] <naderman> a migration can specify what happens on rollback | |
[14:20:54] <nickvergessen> yeah | |
[14:20:57] <naderman> so it should specify how to create the table on rollback | |
[14:21:07] <nickvergessen> but it doesnt know about additional columns from other extensions | |
[14:21:14] <naderman> oh I see | |
[14:21:23] <naderman> now I get it | |
[14:21:42] <naderman> imkingdavid: there's no point in preventing extensions from doing that | |
[14:21:50] <naderman> you'll end up with the same kind of problems with other tables etc. | |
[14:22:01] <naderman> really not a good idea to treat extensions somehow differently than core code | |
[14:22:10] <imkingdavid> in what case is an extension going to drop another extension's table? | |
[14:22:24] <nickvergessen> imkingdavid: in we dont care | |
[14:22:27] <nickvergessen> it can happen | |
[14:22:36] <naderman> imkingdavid: addons | |
[14:22:39] <nickvergessen> and reverting this stop should go back to the previous state | |
[14:22:47] <naderman> just like it may drop a core table | |
[14:22:49] <naderman> really doesn't matter | |
[14:22:58] <naderman> they should just be allowed to change in the schema what they want to change | |
[14:23:08] <naderman> nickvergessen: yeah you're right about that part | |
[14:23:17] <naderman> I suppose we need some mechanism for recreating a table | |
[14:23:23] <naderman> although that won't entirely fix it either | |
[14:23:33] <imkingdavid> so we care about schema but not dat | |
[14:23:34] <imkingdavid> data* | |
[14:23:35] <naderman> I mean at that point you are creating all kinds of conflicts that cannot be solved automatically | |
[14:23:47] <imkingdavid> we create an empty table that the extension cannot use anywa | |
[14:23:50] <imkingdavid> how does this help? | |
[14:23:51] <naderman> nickvergessen: imagine you try to apply an add column on a dropped table | |
[14:23:52] <imkingdavid> anyway* | |
[14:23:57] <naderman> I really think we can just ignore this | |
[14:24:02] <naderman> extensions will just have to not do shit | |
[14:24:03] <naderman> ^_^ | |
[14:24:41] <naderman> I think this is part of what we'll just have to see with the alphas | |
[14:24:45] <naderman> and hopefully some real extensions | |
[14:24:50] <naderman> if any of this turns into a real issue | |
[14:24:56] <naderman> but I doubt it will actually come up in a real scenario | |
[14:24:59] <imkingdavid> it's up to the board admin to keep backups anyway |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment