Page MenuHomePhabricator

Replace.py does not catch exceptions when saving
Closed, ResolvedPublic

Description

python scripts/replace.py -page:User:Kathleen.wright5 wright Dummy -summary:test -lang:en -family:wikisource

does not raise LockedPage exception for User:Kathleen.wright5 (which is a locked page).

Event Timeline

Mpaa raised the priority of this task from to Needs Triage.
Mpaa updated the task description. (Show Details)
Mpaa added a project: Pywikibot.
Mpaa subscribed.
Restricted Application added subscribers: Aklapper, Unknown Object (MLST). · View Herald TranscriptMay 10 2015, 4:57 PM
jayvdb triaged this task as High priority.Jun 9 2015, 6:09 AM
jayvdb added a project: Pywikibot-replace.py.
jayvdb subscribed.

@Mpaa there are 2 modes for calling replace.py in current source code with and without -always argument.

This one without -always (I assume that is Your usercase) require user input in CLI, so it looks fully resonable to update pages asynchronously and in fact replace.py just do that. The main question here is what we should do with raised exception (assuming that You have manually processed much more pages then framework could saved. so there are some already approved and haven't been uploaded to Mediawiki).

OK, so after resolving this issue (what to do with queue of pages to be uploaded to Mediawiki after manual aproval) few technical things to be resolved (for two modes):

  1. with -always
    • there is actually no attempt made to update page, if page.canBeEdited return False (see: 1, it is in replace.py since porting it to pywikibot-core 2)
    • saving here is done with callback (see: 3, callback added in: 4), which will not allow to raise any error during page.save (see: 5, crucial part if not callback added here: 6)
    • the most sane way to raise this exception here is to change callback from _count_changes to something different, that will re-raise exception passed as second parameter.
  2. without -always
    • how to raise exception from other thread? virtually impossible and actually controversial here.

@Xqt what do You think about it? Is this aproach with changing callback to re-raise exception the proper way to go?

Change 322145 had a related patch set uploaded (by Magul):
Raise error when raplace is done synchronously

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

Change 322145 merged by jenkins-bot:
Raise error when replace is done synchronously

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