Awight has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/346583 )

Change subject: Include logfile path in failmails
......................................................................

Include logfile path in failmails

Change-Id: I919594c5c1c0f5316038d6ae63267806092c433e
---
M processcontrol/mailer.py
M processcontrol/runner.py
M tests/test_job_runner.py
3 files changed, 16 insertions(+), 11 deletions(-)


  git pull 
ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/process-control 
refs/changes/83/346583/1

diff --git a/processcontrol/mailer.py b/processcontrol/mailer.py
index 2d74472..8311b4e 100644
--- a/processcontrol/mailer.py
+++ b/processcontrol/mailer.py
@@ -4,15 +4,18 @@
 
 
 class Mailer(object):
-    def __init__(self, config):
-        self.from_address = config.get("failmail/from_address")
-        self.to_address = config.get("failmail/to_address")
+    def __init__(self, job, runner):
+        self.job = job
+        self.runner = runner
+        self.from_address = job.config.get("failmail/from_address")
+        self.to_address = job.config.get("failmail/to_address")
         # FIXME: this is set to ensure one failmail per instance. Should
         # do something more sophisticated to collect all calls and send
         # the mail before exiting.
         self.sent_fail_mail = False
 
-    def fail_mail(self, subject, body="Hope your wits are freshly sharpened!"):
+    def fail_mail(self, subject):
+        body = "See the logs for more information: 
{logfile}".format(logfile=self.runner.logfile)
         if self.sent_fail_mail:
             return
 
diff --git a/processcontrol/runner.py b/processcontrol/runner.py
index 81aaabb..82fa6fc 100644
--- a/processcontrol/runner.py
+++ b/processcontrol/runner.py
@@ -15,7 +15,8 @@
     def __init__(self, job):
         self.global_config = config.GlobalConfiguration()
         self.job = job
-        self.mailer = mailer.Mailer(self.job.config)
+        self.mailer = mailer.Mailer(self.job, self)
+        self.logfile = None
 
     def run(self):
         # Check that we are the service user.
@@ -61,6 +62,7 @@
 
         self.process = subprocess.Popen(command, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE, env=self.job.environment)
         streamer = output_streamer.OutputStreamer(self.process, self.job.slug, 
self.start_time)
+        self.logfile = streamer.filename
         streamer.start()
 
         # should be safe from deadlocks because our OutputStreamer
@@ -76,14 +78,14 @@
         self.process = None
 
     def fail_exitcode(self, return_code):
-        message = "Job {name} failed with code 
{code}".format(name=self.job.name, code=return_code)
+        message = "{name} failed with code {code}".format(name=self.job.name, 
code=return_code)
         config.log.error(message)
         # TODO: Prevent future jobs according to config.
         self.mailer.fail_mail(message)
         raise JobFailure(message)
 
     def fail_has_stderr(self, stderr_data):
-        message = "Job {name} printed things to 
stderr:".format(name=self.job.name)
+        message = "{name} printed things to stderr:".format(name=self.job.name)
         config.log.error(message)
         body = stderr_data.decode("utf-8")
         config.log.error(body)
@@ -92,7 +94,7 @@
 
     def fail_timeout(self):
         self.process.kill()
-        message = "Job {name} timed out after {timeout} 
minutes".format(name=self.job.name, timeout=self.job.timeout)
+        message = "{name} timed out after {timeout} 
minutes".format(name=self.job.name, timeout=self.job.timeout)
         config.log.error(message)
         self.mailer.fail_mail(message)
         # FIXME: Job will return SIGKILL now, fail_exitcode should ignore that 
signal now?
diff --git a/tests/test_job_runner.py b/tests/test_job_runner.py
index a82e808..5af7fac 100644
--- a/tests/test_job_runner.py
+++ b/tests/test_job_runner.py
@@ -51,7 +51,7 @@
         run_job("return_code")
 
     loglines = caplog.actual()
-    assert ("root", "ERROR", "Job False job failed with code 1") in loglines
+    assert ("root", "ERROR", "False job failed with code 1") in loglines
 
     MockSmtp().sendmail.assert_called_once()
 
@@ -65,8 +65,8 @@
         run_job("timeout")
 
     loglines = caplog.actual()
-    assert ("root", "ERROR", "Job Timing out job timed out after 0.005 
minutes") in loglines
-    assert ("root", "ERROR", "Job Timing out job failed with code -9") in 
loglines
+    assert ("root", "ERROR", "Timing out job timed out after 0.005 minutes") 
in loglines
+    assert ("root", "ERROR", "Timing out job failed with code -9") in loglines
 
     MockSmtp().sendmail.assert_called_once()
 

-- 
To view, visit https://gerrit.wikimedia.org/r/346583
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I919594c5c1c0f5316038d6ae63267806092c433e
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/fundraising/process-control
Gerrit-Branch: master
Gerrit-Owner: Awight <awi...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to