Last modified: 2014-08-02 07:21:42 UTC
Problem: When I tried to dump database while the path to gzip executable was misconfigured, script didn’t exit immediately (since it cannot continue in a meaningful way), only printed the error and continued along, even though it could not do anything. Explanation: Looking into the current error handling, I saw it is set up to pass pass exceptions to python default exception handler. That is sufficient only when errors are non-fatal. But when fatal error occurs, if the script does not exit immediately, it starts spewing non-relevant error information and ultimately confuses the user. Proposed solution: Create another error handler (exception handler) for fatal errors, that would output the error message and exit the script on spot. In the attachment is a simple implementation of error handler for fatal errors as well as fix for the issue described above on the spot where it was noticed.
Created attachment 12242 [details] Added error handler for fatal errors in worker.py
Hi! Thanks for your patch! You are welcome to use Developer access https://www.mediawiki.org/wiki/Developer_access to submit this as a Git branch directly into Gerrit: https://www.mediawiki.org/wiki/Git/Tutorial Putting your branch in Git makes it easier to review it quickly. Thanks again! We appreciate your contribution.
I don't think __str__ should be doing the exit. It should probably not have side effects at all, so I don't think this is quite the right approach.
Things like 'gzip isn't in the right place' really ought to be flagged well before we get to running individual dump steps. It would be good to check all config file settings for sanity after they are read in.
Thank you all for your comments. The proposed solution was the first thing that came on my mind, so I do know that it wasn't the best. These days, I'm reading the code again, and searching the right spot for checking conf settings (the idea is to make a function which will call itself during the initialization).
Created attachment 12282 [details] Configurations checks. I've considered what you said, so I've made another patch for worker.py. It now checks for external programs existence in the initialization part. Left to be done/discuss is what to do with checks that already exist, because they are now superfluous.
Sanja: Any chance you could put that into Gerrit? See comment 2.
It is in Gerrit. Gerrit Change-Id: I3f7fafa6d2813931bf328bba6c519029ea412c99
Thanks! (Adding "bug: 48012" as the last line in the patch commit message line will create an automatic comment in this bug report)
Ah please see my comments on the changeset, over on the gerrit side.
Related URL: https://gerrit.wikimedia.org/r/63782 (Gerrit Change Ied6711771d4db7aae6858f1fccf0020e8297b4c7)
Change 63390 had a related patch set uploaded by Nemo bis: Per bug #48012. Patch for worker.py. It checks for external programs existence in the initialization part. https://gerrit.wikimedia.org/r/63390
Not a MediaWiki issue...
