Page MenuHomePhabricator

Merge scap scripts "mwscriptwikiset" and "foreachwikiindblist" into one
Open, Needs TriagePublic

Description

<Krenair> hey RoanKattouw, do you remember why we have both foreachwikiindblist and mwscriptwikiset?
<RoanKattouw> Krenair: No, I don't know
<RoanKattouw> Krenair: foreachwikiindblist takes a newline-separated list of wikis in a text file, amybe mwscriptwikiset takes its list in some other form?
<Krenair> parameters are opposite ways around
<Krenair> foreachwikiindblist tries to read dblist from current directory
<Krenair> mwscriptwikiset forces them to be relative to $MEDIAWIKI_DEPLOYMENT_DIR, and makes Ctrl-C work
<RoanKattouw> Oh OK
<RoanKattouw> So it's a more useful version of foreachwikiindblist
<RoanKattouw> Then we should probably remove foreachwikiindblist and alias it to mwscriptwikiset
<Krenair> unless you have a dblist in the current directory you want to use
<Krenair> but <Krenair> parameters are opposite ways around
<Krenair> mwscriptwikiset scriptfile listfile vs. foreachwikiindblist listfile scriptfile
<RoanKattouw> Urgh
<RoanKattouw> You could reduce foreachwikiindblist to a wrapper script around mwscriptwikiset that just flips the arguments I suppose?
<RoanKattouw> But yeah you might also have dblists in your home dir or something
<Krenair> these are 4 or more years old - https://gerrit.wikimedia.org/r/559

See also T101213 - we probably want it to try to load from /srv/mediawiki-staging, then /srv/mediawiki, then the current directory before giving up?

Event Timeline

Krenair raised the priority of this task from to Needs Triage.
Krenair updated the task description. (Show Details)
Krenair added a project: acl*sre-team.
Krenair added subscribers: Krenair, bd808, Catrope.

Another difference between the two scripts is that foreachwikiindblist uses expanddblist to support list files which use the dblist expression language ("list + list - list") while mwscriptwikiset only deals with plain files. This means that mwscriptwikiset won't work with the group1.dblist and and flow_computed.dblist files in mw-config.

The main improvement I can see in mwscriptwikiset is the Ctrl-C handling.

mwscriptwikiset seems to be used in a number of Puppetized files:

  • modules/mediawiki/files/maintenance/update-article-count
  • modules/mediawiki/manifests/maintenance/refreshlinks/cronjob.pp
  • modules/mediawiki/manifests/maintenance/update_flaggedrev_stats.pp
  • modules/mediawiki/manifests/maintenance/updatequerypages/cronjob.pp
  • modules/mediawiki/manifests/maintenance/updatequerypages/enwiki/cronjob.pp

foreachwikiindblist is used indirectly via foreachwiki in these Puppet-managed files:

  • modules/mediawiki/manifests/maintenance/cleanup_upload_stash.pp
  • modules/mediawiki/manifests/maintenance/echo_mail_batch.pp
  • modules/mediawiki/manifests/maintenance/purge_abusefilter.pp
  • modules/mediawiki/manifests/maintenance/purge_checkuser.pp
  • modules/mediawiki/manifests/maintenance/purge_securepoll.pp
  • modules/mediawiki/manifests/maintenance/update_special_pages.pp
  • modules/scap/files/l10nupdate-1

I don't know which of the foreachwiki/foreachwikiindblist and mwscriptwikiset competing scripts is most used manually.

I don't find any direct usage of foreachwikiindblist in Puppet, only the indirect usage via foreachwiki, so I might be safe to combine the two scripts and use the argument order of mwscriptwikiset. In a combined script I would suggest taking foreachwikiindblist as the base and adding the Ctrl-C handling from mwscriptwikiset. The ability to use arbitrary lists in foreachwikiindblist is probably worth preserving. I think that can be reconciled with the resolution against "$MEDIAWIKI_DEPLOYMENT_DIR/dblists" that is done by mwscriptwikiset in a combined implementation.