[13:06:31] (03CR) 10Thiemo Mättig (WMDE): "Why is this merged? I'm sorry that my comments are not understood. I will try again. To summarize again: This script is broken and can not" (034 comments) [extensions/ORES] - 10https://gerrit.wikimedia.org/r/312286 (https://phabricator.wikimedia.org/T145503) (owner: 10Ladsgroup) [13:07:32] (03PS1) 10Thiemo Mättig (WMDE): Revert "Add maintenance/CleanDuplicateScores.php" [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314258 (https://phabricator.wikimedia.org/T145503) [13:14:08] (03CR) 10Ladsgroup: "Hey, why?" [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314258 (https://phabricator.wikimedia.org/T145503) (owner: 10Thiemo Mättig (WMDE)) [13:26:16] (03CR) 10Thiemo Mättig (WMDE): "As I wrote in the commit message: See my comments in I9ea60cd for the reasons." [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314258 (https://phabricator.wikimedia.org/T145503) (owner: 10Thiemo Mättig (WMDE)) [14:00:58] (03CR) 10Ladsgroup: "It's a maintenance script. Let's fix it instead of reverting." [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314258 (https://phabricator.wikimedia.org/T145503) (owner: 10Thiemo Mättig (WMDE)) [14:02:47] (03CR) 10Thiemo Mättig (WMDE): "Sure, both is fine. This patch is more a reminder. You can use and change it or just open a new one. I will abandon this then." [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314258 (https://phabricator.wikimedia.org/T145503) (owner: 10Thiemo Mättig (WMDE)) [15:25:00] (03PS1) 10Ladsgroup: Fixup maintenance/CleanDuplicateScores.php [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 [15:26:26] (03CR) 10jenkins-bot: [V: 04-1] Fixup maintenance/CleanDuplicateScores.php [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 (owner: 10Ladsgroup) [17:20:31] (03PS2) 10Ladsgroup: Fixup maintenance/CleanDuplicateScores.php [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 [17:21:15] (03CR) 10Thiemo Mättig (WMDE): [C: 031] "Looks very good. Thanks a lot for the patch. Feel free to merge." [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 (owner: 10Ladsgroup) [17:21:48] (03Abandoned) 10Thiemo Mättig (WMDE): Revert "Add maintenance/CleanDuplicateScores.php" [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314258 (https://phabricator.wikimedia.org/T145503) (owner: 10Thiemo Mättig (WMDE)) [17:23:02] (03CR) 10Ladsgroup: "It still needs some clean up. I'm getting it done ASAP." [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 (owner: 10Ladsgroup) [18:33:44] (03PS3) 10Ladsgroup: Fixup maintenance/CleanDuplicateScores.php [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 [18:38:06] (03CR) 10Ladsgroup: "Now, it's ready to merge. Tested in mw-revscoring.wmflabs.org" [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 (owner: 10Ladsgroup) [19:56:38] halfak: hey, do you have a minute to merge a patch? [19:56:49] Yeah! linky? [19:57:02] sure [19:57:20] https://gerrit.wikimedia.org/r/314281 [20:00:58] Amir1, I'm struggling a bit to see how the query is getting built. [20:01:17] halfak: It uses mediawiki abstraction layer [20:01:27] which is complicated on its own [20:01:48] specially group_concat [20:02:10] Yeah. Trying to work out what it does [20:02:16] What's the "|" for? [20:02:29] And why are we exploding "|" too? [20:02:59] it's the delimiter [20:03:17] oresc_rev = 1|2|3 [20:06:12] It's building this, right? http://dev.mysql.com/doc/refman/5.7/en/group-by-functions.html#function_group-concat [20:06:20] if so, what do the arguments mean? [20:09:14] Amir1, ^ [20:09:40] let me grab the manual in mediawiki [20:09:55] OMG IT HAS IT'S OWN TABLE REFERENCE [20:09:56] WHY [20:10:02] https://doc.wikimedia.org/mediawiki-core/REL1_25/php/classDatabaseOracle.html [20:10:23] halfak: https://doc.wikimedia.org/mediawiki-core/1.27.0/php/classDatabaseBase.html#a81b5d54abfcc999092129dd6f88b9ae8 [20:10:55] BURN IT DOWN [20:10:59] * halfak gets back to reviewing [20:12:29] halfak: Finally it will build this: ""SELECT oresc_rev,oresc_model,oresc_class,(SELECT GROUP_CONCAT(ores_classification.oresc_id SEPARATOR '|') FROM ores_classification AS OC WHERE OC.oresc_id = ores_classification.oresc_id ) AS `ids` FROM `ores_classification` GROUP BY oresc_rev, oresc_model, oresc_class HAVING COUNT(*) > 1" [20:13:42] OK. Looks like I have only one concern. it looks like it'll pass the oresc_ids back in as strings for deletion [20:14:08] I guess MySQL will do the conversion for you [20:16:49] halfak: mediawiki will handle it [20:17:15] When you tested it, were there duplicates that you confirmed were removed? [20:17:28] yes, I tested it in mw-revscoring.wmflabs.org [20:17:48] (fun fact: I brought it down for one hour trying to update it :D) [20:18:23] (03CR) 10Halfak: [C: 032] Fixup maintenance/CleanDuplicateScores.php [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 (owner: 10Ladsgroup) [20:18:49] OK. Sorry for all of the questions. [20:18:56] MediaWiki is dense sometimes [20:19:14] (03Merged) 10jenkins-bot: Fixup maintenance/CleanDuplicateScores.php [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314281 (owner: 10Ladsgroup) [20:19:42] no problem :) It's healthy [22:56:41] (03PS1) 10Ladsgroup: Expose ores scores in js [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314449 (https://phabricator.wikimedia.org/T143611) [22:57:50] (03CR) 10jenkins-bot: [V: 04-1] Expose ores scores in js [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314449 (https://phabricator.wikimedia.org/T143611) (owner: 10Ladsgroup) [23:01:59] (03PS2) 10Ladsgroup: Expose ores scores in js [extensions/ORES] - 10https://gerrit.wikimedia.org/r/314449 (https://phabricator.wikimedia.org/T143611)