Page MenuHomePhabricator

Create maintenance script to properly mark old autopatrol actions in logging table
Closed, DeclinedPublic

Description

@Ladsgroup That seems fine, but it might also make sense for this to be more re-usable and less manual/error-prone work. For example, we could have a maintenance script that goes through logging/patrol/patrol<2017 (as you mentioned) but instead of deleting things, it could SET log_action='autopatrol' for the ones it finds.
Then once that's done, we can run the autopatrol-purge script without any special parameters.
Benefits:

  • The purge script will only need to focus on autopatrol.
  • We'll be able to re-use it for (eventually, low-priority) correcting older logs on other wikis to provide filtering and separation of patrol vs autopatrol.
  • We'll be able to re-use it when we want to do purging on another wiki in the future. (Without manual queries and deletion.)
  • It'll be usable by third-parties.
  • We could even add the patrol-marker script to the installer by default to run once on upgrades, so that small/third-party wikis will have all old autopatrol records properly marked as such.

Event Timeline

Ladsgroup triaged this task as High priority.Mar 22 2018, 8:04 PM
Ladsgroup created this task.
Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptMar 22 2018, 8:04 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Krinkle updated the task description. (Show Details)Mar 22 2018, 9:09 PM
Ladsgroup closed this task as Declined.Mar 23 2018, 1:24 AM

Implemented in https://gerrit.wikimedia.org/r/#/c/420214/10..11/maintenance/deleteAutoPatrolLogs.php,unified
I almost was finished with this:
https://gist.github.com/Ladsgroup/d54fc12fdc311c695e1c7e14934cfa62
Then I realized there is no need to check it against the revision table and the data exists in log_params.

Krinkle added a comment.EditedMar 26 2018, 11:40 PM

@Ladsgroup I'm not sure how this relates to revision. Patrol logs have always varied between manual actions and automatic self-actions. The only difference is that pre-2016, it was just a variable in the interface message based on the serialised params blob. After that, in 2016 we introduce patrol/autopatrol as separate type-action pair for autopatrols from the regular patrol/patrol. This script would be to batch through the old patrol/patrol logs, and where the serialised blobs contains auto and is true, update log_action to be autopatrol. Basically, to correct the old logs.

We can then run that script separately from the purge script ahead of time, potentially also run the converter on other wikis where we don't want to do the purge. Plus, we can add the conversion script to the installer as an upgrade script for to help third-parties correct their logs as well.

All of this only makes sense, however, if we plan to keep support for autopatrol logs in some way. Because separating the scripts, allows us to run the conversion cleanup on all wikis, and for third-parties, whilst only disabling/purging on some wikis.

But, we have already approved the parent task (T184485) that intends to remove autopatrol logging more permanently. Not configurable. And that makes sense, given they don't add value, especially once the recentchanges issue is fixed to keep the distinction there.

As such, I agree we don't need the separation. We only need one script, and we should run it everywhere (eventually). And also part of the installer/upgrader, like we do for other cleanup/conversation scripts.


Let's continue at T189594.