Page MenuHomePhabricator

CirrusSearch:UpdateWeightedTags throws when called with --reset
Closed, ResolvedPublic

Description

Recently, I tried using the CirrusSearch:UpdateWeightedTags maint. script to reset weighed tags for a page in my local environment:

I have no name!@9fdafa6fa954:/var/www/html/w$ php maintenance/run.php CirrusSearch:UpdateWeightedTags --reset --page=Stát --tagType=recommendation.image --verbose
[bb5b62cb4c3ee405c3649559] [no req]   TypeError: CirrusSearch\Updater::resetWeightedTags(): Argument #2 ($tagPrefixes) must be of type array, string given, called in /var/www/html/w/extensions/CirrusSearch/maintenance/UpdateWeightedTags.php on line 64
Backtrace:
from /var/www/html/w/extensions/CirrusSearch/includes/Updater.php(281)
#0 /var/www/html/w/extensions/CirrusSearch/maintenance/UpdateWeightedTags.php(64): CirrusSearch\Updater->resetWeightedTags()
#1 /var/www/html/w/maintenance/includes/MaintenanceRunner.php(696): CirrusSearch\Maintenance\UpdateWeightedTags->execute()
#2 /var/www/html/w/maintenance/run.php(53): MediaWiki\Maintenance\MaintenanceRunner->run()
#3 {main}
I have no name!@9fdafa6fa954:/var/www/html/w$

It seems there is an inconsistency between the expectations regarding $tagPrefix of WeighedTagsUpdater::resetWeightedTags() (which expects an array) and CirrusSearch:UpdateWeightedTags (which sends a singular string). This is a difference from WeighedTagsUpdater::updateWeightedTags() (which only accepts a singular prefix).

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Based on my research, allowing multiple prefixes in the maint. script makes no sense (it would also cause issues with updating, rather than resetting). Using [ $tagPrefix ] makes the most sense, IMO.

Change #1183718 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/CirrusSearch@master] UpdateWeighedTags: Support --reset operation

https://gerrit.wikimedia.org/r/1183718

The patch I uploaded fails on CI, because CirrusSearch currently fails on master. I logged T403427: CirrusSearch tests fail on Phan with master.

Discovery-Search: If you could help with writing tests to verify the script at the very least does not throw, this would be appreciated.

Change #1183718 merged by jenkins-bot:

[mediawiki/extensions/CirrusSearch@master] UpdateWeighedTags: Support --reset operation

https://gerrit.wikimedia.org/r/1183718