Skip to content

Commit 3e771d5

Browse files
charles-cowartcharles-cowart
authored
Fix #3230 (#3244)
* Status change emails now more informative * Updates based on feedback * Unit test added * Updates based on feedback
1 parent bda13f8 commit 3e771d5

File tree

2 files changed

+194
-17
lines changed

2 files changed

+194
-17
lines changed

‎qiita_db/processing_job.py

Lines changed: 101 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ def create(cls, user, parameters, force=False):
594594
if vals[0] == 'artifact':
595595
artifact_info = parameters.values[pname]
596596
# If the artifact_info is a list, then the artifact
597-
# still doesn't exists because the current job is part
597+
# still doesn't exist because the current job is part
598598
# of a workflow, so we can't link
599599
if not isinstance(artifact_info, list):
600600
TTRN.add(sql, [artifact_info, job_id])
@@ -703,6 +703,101 @@ def status(self):
703703
qdb.sql_connection.TRN.add(sql, [self.id])
704704
return qdb.sql_connection.TRN.execute_fetchlast()
705705

706+
def _generate_notification_message(self, value, error_msg):
707+
ignored_software = ('artifact definition',)
708+
ignored_commands = ('Validate', 'complete_job', 'release_validators')
709+
710+
# abort early conditions (don't send an email notification)
711+
# tentatively accept the overhead of a function-call, even when a
712+
# notification isn't sent, just to keep the logic clean and
713+
# centralized.
714+
715+
if value == 'waiting':
716+
# notification not needed.
717+
return None
718+
719+
if not self.user.info['receive_processing_job_emails']:
720+
# notification not needed.
721+
return None
722+
723+
if self.command.software.name in ignored_software:
724+
# notification not needed.
725+
return None
726+
727+
if self.command.name in ignored_commands:
728+
# notification not needed.
729+
return None
730+
731+
# generate subject line
732+
subject = 'Job status change: %s (%s)' % (self.command.name, self.id)
733+
734+
# generate message line
735+
message = ''
736+
737+
input_artifacts = self.input_artifacts
738+
if input_artifacts is None:
739+
# this is an admin job. display command name and parameters
740+
message = (f'Admin Job {self.command.name} '
741+
f'{self.command.parameters}')
742+
else:
743+
for artifact in input_artifacts:
744+
if artifact.prep_templates is not None:
745+
# this is a processing job. display the study id as link,
746+
# prep ids, data_type, and command name.
747+
study_ids = [x.study_id for x in artifact.prep_templates]
748+
prep_ids = [x.id for x in artifact.prep_templates]
749+
data_types = [x.data_type() for x in
750+
artifact.prep_templates]
751+
752+
# there should only be one study id
753+
study_ids = set(study_ids)
754+
if len(study_ids) > 1:
755+
raise qdb.exceptions.QiitaError("More than one Study "
756+
"ID was found: "
757+
f"{study_ids}")
758+
study_id = study_ids.pop()
759+
760+
# there should be at least one prep_id and probably more.
761+
prep_ids = set(prep_ids)
762+
if len(prep_ids) == 0:
763+
raise qdb.exceptions.QiitaError("No Prep IDs were "
764+
"found")
765+
# convert into a string for presentation.
766+
prep_ids = [str(x) for x in prep_ids]
767+
prep_ids = ', '.join(prep_ids)
768+
769+
# there should be only one data type.
770+
data_types = set(data_types)
771+
if len(data_types) > 1:
772+
raise qdb.exceptions.QiitaError("More than one data "
773+
"type was found: "
774+
f"{data_types}")
775+
data_type = data_types.pop()
776+
777+
message = f'Processing Job: {self.command.name}\n'
778+
message += 'Study <A HREF="https://qiita.ucsd.edu/study/'
779+
message += f'description/{study_id}">{study_id}'
780+
message += '</A>\n'
781+
message += f'Prep IDs: {prep_ids}\n'
782+
message += f'Data Type: {data_type}\n'
783+
elif artifact.analysis is not None:
784+
# this is an analysis job. display analysis id as link and
785+
# the command name.
786+
message = f'Analysis Job {self.command.name} '
787+
message += '<A HREF="https://qiita.ucsd.edu/analysis/'
788+
message += f'description/{artifact.analysis.id}">'
789+
message += f'{artifact.analysis.id}</A>\n'
790+
else:
791+
raise qdb.exceptions.QiitaError("Unknown Condition")
792+
793+
# append legacy message line
794+
message += 'New status: %s' % (value)
795+
796+
if value == 'error' and error_msg is not None:
797+
message += f'\n\nError:\n{error_msg}'
798+
799+
return {'subject': subject, 'message': message}
800+
706801
def _set_status(self, value, error_msg=None):
707802
"""Sets the status of the job
708803
@@ -734,22 +829,11 @@ def _set_status(self, value, error_msg=None):
734829
new_status = qdb.util.convert_to_id(
735830
value, "processing_job_status")
736831

737-
if value not in {'waiting'}:
738-
if self.user.info['receive_processing_job_emails']:
739-
# skip if software is artifact definition
740-
ignore_software = ('artifact definition', )
741-
if self.command.software.name not in ignore_software:
742-
ignore_commands = ('Validate', 'complete_job',
743-
'release_validators')
744-
if self.command.name not in ignore_commands:
745-
subject = 'Job status change: %s (%s)' % (
746-
self.command.name, self.id)
747-
message = 'New status: %s' % (value)
748-
749-
if value == 'error' and error_msg is not None:
750-
message += f'\n\nError:\n{error_msg}'
751-
qdb.util.send_email(
752-
self.user.email, subject, message)
832+
msg = self._generate_notification_message(value, error_msg)
833+
if msg is not None:
834+
# send email
835+
qdb.util.send_email(self.user.email, msg['subject'],
836+
msg['message'])
753837

754838
sql = """UPDATE qiita.processing_job
755839
SET processing_job_status_id = %s

‎qiita_db/test/test_processing_job.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ def setUp(self):
6969
"b72369f9-a886-4193-8d3d-f7b504168e75")
7070
self.tester4 = qdb.processing_job.ProcessingJob(
7171
"d19f76ee-274e-4c1b-b3a2-a12d73507c55")
72+
7273
self._clean_up_files = []
7374

7475
def _get_all_job_ids(self):
@@ -844,6 +845,98 @@ def _set_allocation(memory):
844845
self.assertEqual(job_changed.get_resource_allocation_info(),
845846
'-p qiita --mem 2G')
846847

848+
def test_notification_mail_generation(self):
849+
# Almost all processing-jobs in testing are owned by [email protected]
850+
# and are of type 'Split libraries FASTQ'.
851+
852+
# as '[email protected]' is not set to receive notifications, let's
853+
# first manually set their configuration to 'true'.
854+
sql = ("UPDATE qiita.qiita_user SET receive_processing_job_emails"
855+
" = true WHERE email = '[email protected]'")
856+
857+
with qdb.sql_connection.TRN:
858+
qdb.sql_connection.TRN.add(sql)
859+
860+
# with or w/out an error message, a status of 'waiting' should
861+
# immediately return with a 'None' message.
862+
obs = self.tester1._generate_notification_message('waiting', None)
863+
self.assertEqual(obs, None)
864+
obs = self.tester1._generate_notification_message('waiting',
865+
'Hello World')
866+
self.assertEqual(obs, None)
867+
868+
# An error message in the parameter should show a difference for
869+
# messages of type 'error'.
870+
obs = self.tester1._generate_notification_message('error', None)
871+
872+
exp = {'subject': ('Job status change: Split libraries FASTQ '
873+
'(063e553b-327c-4818-ab4a-adfe58e49860)'),
874+
'message': ('Processing Job: Split libraries FASTQ\nStudy '
875+
'<A HREF="https://qiita.ucsd.edu/study/description'
876+
'/1">1</A>\nPrep IDs: 1\nData Type: 18S\nNew '
877+
'status: error')}
878+
879+
self.assertDictEqual(obs, exp)
880+
881+
obs = self.tester1._generate_notification_message('error',
882+
'An Error Message')
883+
884+
exp = {'subject': ('Job status change: Split libraries FASTQ '
885+
'(063e553b-327c-4818-ab4a-adfe58e49860)'),
886+
'message': ('Processing Job: Split libraries FASTQ\nStudy '
887+
'<A HREF="https://qiita.ucsd.edu/study/description'
888+
'/1">1</A>\nPrep IDs: 1\nData Type: 18S\nNew status'
889+
': error\n\nError:\nAn Error Message')}
890+
891+
self.assertDictEqual(obs, exp)
892+
893+
# The inclusion of an error message has no effect on other valid
894+
# status types e.g. 'running'.
895+
obs = self.tester1._generate_notification_message('running', None)
896+
897+
exp = {'subject': ('Job status change: Split libraries FASTQ '
898+
'(063e553b-327c-4818-ab4a-adfe58e49860)'),
899+
'message': ('Processing Job: Split libraries FASTQ\nStudy '
900+
'<A HREF="https://qiita.ucsd.edu/study/description'
901+
'/1">1</A>\nPrep IDs: 1\nData Type: 18S\nNew status'
902+
': running')}
903+
904+
self.assertDictEqual(obs, exp)
905+
906+
obs = self.tester1._generate_notification_message('running', 'Yahoo!')
907+
908+
exp = {'subject': ('Job status change: Split libraries FASTQ '
909+
'(063e553b-327c-4818-ab4a-adfe58e49860)'),
910+
'message': ('Processing Job: Split libraries FASTQ\nStudy '
911+
'<A HREF="https://qiita.ucsd.edu/study/description'
912+
'/1">1</A>\nPrep IDs: 1\nData Type: 18S\nNew status'
913+
': running')}
914+
915+
self.assertDictEqual(obs, exp)
916+
917+
# as '[email protected]' is not set to receive notifications, let's
918+
# first manually set their configuration to 'true'.
919+
# reset [email protected] to 'false' to test expectations for a non-
920+
# privileged user.
921+
sql = ("UPDATE qiita.qiita_user SET receive_processing_job_emails"
922+
" = false WHERE email = '[email protected]'")
923+
924+
with qdb.sql_connection.TRN:
925+
qdb.sql_connection.TRN.add(sql)
926+
927+
# waiting should still return w/out a message.
928+
obs = self.tester1._generate_notification_message('waiting', None)
929+
self.assertEqual(obs, None)
930+
931+
# an error status should now return nothing.
932+
obs = self.tester1._generate_notification_message('error',
933+
'An Error Message')
934+
self.assertEqual(obs, None)
935+
936+
# other status messages should also return nothing.
937+
obs = self.tester1._generate_notification_message('running', None)
938+
self.assertEqual(obs, None)
939+
847940

848941
@qiita_test_checker()
849942
class ProcessingWorkflowTests(TestCase):

0 commit comments

Comments
 (0)