From 49be285a521139a6214b93e592d86af9c539b77a Mon Sep 17 00:00:00 2001 From: Marcin Apostoluk Date: Mon, 15 May 2023 12:50:59 +0100 Subject: [PATCH 1/4] error email improvements --- notebooker/constants.py | 12 +++++-- notebooker/execute_notebook.py | 18 ++++++++--- notebooker/serialization/mongo.py | 5 +++ notebooker/utils/notebook_execution.py | 24 +++++++++----- notebooker/web/routes/report_execution.py | 6 ++++ notebooker/web/routes/scheduling.py | 2 ++ notebooker/web/scheduler.py | 3 ++ notebooker/web/static/notebooker/scheduler.js | 3 ++ notebooker/web/templates/results.html | 3 ++ notebooker/web/templates/run_report.html | 13 ++++++-- notebooker/web/templates/scheduler.html | 6 ++++ tests/integration/test_e2e.py | 8 +++++ tests/integration/test_execute_notebook.py | 31 +++++++------------ tests/integration/web/test_run_report.py | 3 ++ tests/integration/web/test_scheduling.py | 3 ++ tests/unit/test_run_report.py | 1 + 16 files changed, 104 insertions(+), 37 deletions(-) diff --git a/notebooker/constants.py b/notebooker/constants.py index edbd9902..6559ae5f 100644 --- a/notebooker/constants.py +++ b/notebooker/constants.py @@ -80,6 +80,7 @@ class NotebookResultBase(object): status = attr.ib(default=JobStatus.ERROR) overrides = attr.ib(default=attr.Factory(dict)) mailto = attr.ib(default="") + error_mailto = attr.ib(default="") generate_pdf_output = attr.ib(default=True) hide_code = attr.ib(default=False) stdout = attr.ib(default=attr.Factory(list)) @@ -100,6 +101,7 @@ class NotebookResultPending(NotebookResultBase): report_title = attr.ib(default="") overrides = attr.ib(default=attr.Factory(dict)) mailto = attr.ib(default="") + error_mailto = attr.ib(default="") generate_pdf_output = attr.ib(default=True) hide_code = attr.ib(default=False) scheduler_job_id = attr.ib(default=None) @@ -115,6 +117,7 @@ class NotebookResultError(NotebookResultBase): report_title = attr.ib(default="") overrides = attr.ib(default=attr.Factory(dict)) mailto = attr.ib(default="") + error_mailto = attr.ib(default="") generate_pdf_output = attr.ib(default=True) hide_code = attr.ib(default=False) scheduler_job_id = attr.ib(default=None) @@ -155,6 +158,7 @@ class NotebookResultComplete(NotebookResultBase): report_title = attr.ib(default="") overrides = attr.ib(default=attr.Factory(dict)) mailto = attr.ib(default="") + error_mailto = attr.ib(default="") email_subject = attr.ib(default="") generate_pdf_output = attr.ib(default=True) hide_code = attr.ib(default=False) @@ -185,6 +189,7 @@ def saveable_output(self): "job_start_time": self.job_start_time, "job_finish_time": self.job_finish_time, "mailto": self.mailto, + "error_mailto": self.error_mailto, "email_subject": self.email_subject, "overrides": self.overrides, "generate_pdf_output": self.generate_pdf_output, @@ -200,9 +205,9 @@ def __repr__(self): return ( "NotebookResultComplete(job_id={job_id}, status={status}, report_name={report_name}, " "job_start_time={job_start_time}, job_finish_time={job_finish_time}, update_time={update_time}, " - "report_title={report_title}, overrides={overrides}, mailto={mailto}, mailfrom={mailfrom}" - "email_subject={email_subject}, generate_pdf_output={generate_pdf_output}, hide_code={hide_code}, " - "scheduler_job_id={scheduler_job_id}, is_slideshow={is_slideshow})".format( + "report_title={report_title}, overrides={overrides}, mailto={mailto}, error_mailto={error_mailto}, " + "mailfrom={mailfrom}, email_subject={email_subject}, generate_pdf_output={generate_pdf_output}, " + "hide_code={hide_code}, scheduler_job_id={scheduler_job_id}, is_slideshow={is_slideshow})".format( job_id=self.job_id, status=self.status, report_name=self.report_name, @@ -212,6 +217,7 @@ def __repr__(self): report_title=self.report_title, overrides=self.overrides, mailto=self.mailto, + error_mailto=self.error_mailto, mailfrom=self.mailfrom, email_subject=self.email_subject, generate_pdf_output=self.generate_pdf_output, diff --git a/notebooker/execute_notebook.py b/notebooker/execute_notebook.py index f02ff0d7..f942ebfe 100644 --- a/notebooker/execute_notebook.py +++ b/notebooker/execute_notebook.py @@ -44,6 +44,7 @@ def _run_checks( generate_pdf_output: Optional[bool] = True, hide_code: Optional[bool] = False, mailto: Optional[str] = "", + error_mailto: Optional[str] = "", email_subject: Optional[str] = "", prepare_only: Optional[bool] = False, notebooker_disable_git: bool = False, @@ -76,7 +77,9 @@ def _run_checks( generate_pdf_output : `Optional[bool]` Whether to generate PDF output or not. NB this requires xelatex to be installed on the executor. mailto : `Optional[str]` - Comma-separated email addresses to send on completion (or error). + Comma-separated email addresses to send on completion. + error_mailto : `Optional[str]` + Comma-separated email addresses to send on error. prepare_only : `Optional[bool]` Internal usage. Whether we want to do everything apart from executing the notebook. scheduler_job_id : `Optional[str]` @@ -128,6 +131,7 @@ def _run_checks( raw_html=html, email_html=email_html, mailto=mailto, + error_mailto=error_mailto, email_subject=email_subject, pdf=pdf, generate_pdf_output=generate_pdf_output, @@ -191,6 +195,7 @@ def run_report( template_base_dir, overrides, mailto=mailto, + error_mailto=error_mailto, email_subject=email_subject, generate_pdf_output=generate_pdf_output, hide_code=hide_code, @@ -215,7 +220,8 @@ def run_report( report_title=report_title, error_info=error_info, overrides=overrides, - mailto=error_mailto or mailto, + mailto=mailto, + error_mailto=error_mailto, generate_pdf_output=generate_pdf_output, scheduler_job_id=scheduler_job_id, mailfrom=mailfrom, @@ -402,8 +408,7 @@ def execute_notebook_entrypoint( mailfrom=mailfrom, is_slideshow=is_slideshow, ) - if result.mailto: - send_result_email(result, config.DEFAULT_MAILFROM) + send_result_email(result, config.DEFAULT_MAILFROM) logger.info(f"Here is the result!{result}") if isinstance(result, NotebookResultError): logger.warning("Notebook execution failed! Output was:") @@ -458,6 +463,7 @@ def run_report_in_subprocess( report_name, report_title, mailto, + error_mailto, overrides, *, hide_code=False, @@ -476,6 +482,7 @@ def run_report_in_subprocess( :param report_name: `str` The report which we are executing :param report_title: `str` The user-specified title of the report :param mailto: `Optional[str]` Who the results will be emailed to + :param error_mailto: `Optional[str]` Who the errors will be emailed to :param overrides: `Optional[Dict[str, Any]]` The parameters to be passed into the report :param generate_pdf_output: `bool` Whether we're generating a PDF. Defaults to False. :param prepare_only: `bool` Whether to do everything except execute the notebook. Useful for testing. @@ -497,6 +504,7 @@ def run_report_in_subprocess( status=JobStatus.SUBMITTED, overrides=overrides, mailto=mailto, + error_mailto=error_mailto, generate_pdf_output=generate_pdf_output, hide_code=hide_code, scheduler_job_id=scheduler_job_id, @@ -530,6 +538,8 @@ def run_report_in_subprocess( report_title, "--mailto", mailto, + "--error-mailto", + error_mailto, "--overrides-as-json", json.dumps(overrides), "--pdf-output" if generate_pdf_output else "--no-pdf-output", diff --git a/notebooker/serialization/mongo.py b/notebooker/serialization/mongo.py index fd62b488..1c3b29c0 100644 --- a/notebooker/serialization/mongo.py +++ b/notebooker/serialization/mongo.py @@ -187,6 +187,7 @@ def save_check_stub( status: JobStatus = JobStatus.PENDING, overrides: Optional[Dict] = None, mailto: str = "", + error_mailto: str = "", generate_pdf_output: bool = True, hide_code: bool = False, scheduler_job_id: Optional[str] = None, @@ -202,6 +203,7 @@ def save_check_stub( job_start_time=job_start_time, report_name=report_name, mailto=mailto, + error_mailto=error_mailto, generate_pdf_output=generate_pdf_output, overrides=overrides or {}, hide_code=hide_code, @@ -299,6 +301,7 @@ def _convert_result( generate_pdf_output=result.get("generate_pdf_output", True), report_title=result.get("report_title", result["report_name"]), mailto=result.get("mailto", ""), + error_mailto=result.get("error_mailto", ""), hide_code=result.get("hide_code", False), stdout=result.get("stdout", []), scheduler_job_id=result.get("scheduler_job_id", None), @@ -315,6 +318,7 @@ def _convert_result( generate_pdf_output=result.get("generate_pdf_output", True), report_title=result.get("report_title", result["report_name"]), mailto=result.get("mailto", ""), + error_mailto=result.get("error_mailto", ""), hide_code=result.get("hide_code", False), stdout=result.get("stdout", []), scheduler_job_id=result.get("scheduler_job_id", None), @@ -338,6 +342,7 @@ def _convert_result( generate_pdf_output=result.get("generate_pdf_output", True), report_title=result.get("report_title", result["report_name"]), mailto=result.get("mailto", ""), + error_mailto=result.get("error_mailto", ""), hide_code=result.get("hide_code", False), stdout=result.get("stdout", []), scheduler_job_id=result.get("scheduler_job_id", False), diff --git a/notebooker/utils/notebook_execution.py b/notebooker/utils/notebook_execution.py index b81d9933..c8b0f589 100644 --- a/notebooker/utils/notebook_execution.py +++ b/notebooker/utils/notebook_execution.py @@ -15,12 +15,7 @@ def _output_dir(output_base_dir, report_name, job_id): return os.path.join(output_base_dir, report_name, job_id) -def send_result_email(result: Union[NotebookResultComplete, NotebookResultError], default_mailfrom: str) -> None: - if result.mailfrom: - mailfrom = result.mailfrom - else: - mailfrom = default_mailfrom - to_email = result.mailto +def _send_email(from_email: str, to_email: str, result: Union[NotebookResultComplete, NotebookResultError]) -> None: report_title = ( result.report_title.decode("utf-8") if isinstance(result.report_title, bytes) else result.report_title ) @@ -57,8 +52,23 @@ def send_result_email(result: Union[NotebookResultComplete, NotebookResultError] msg = ["Please either activate HTML emails, or see the PDF attachment.", body] logger.info("Sending email to %s with %d attachments", to_email, len(attachments)) - mail(mailfrom, to_email, subject, msg, attachments=attachments) + mail(from_email, to_email, subject, msg, attachments=attachments) finally: if tmp_dir: logger.info("Cleaning up temporary email attachment directory %s", tmp_dir) shutil.rmtree(tmp_dir) + + +def send_result_email(result: Union[NotebookResultComplete, NotebookResultError], default_mailfrom: str) -> None: + if result.mailfrom: + mailfrom = result.mailfrom + else: + mailfrom = default_mailfrom + if isinstance(result, NotebookResultComplete): + to_email = result.mailto + else: + to_email = result.error_mailto + if not to_email: + logger.info("Not sending email as no recipients specified") + return + _send_email(from_email=mailfrom, to_email=to_email, result=result) diff --git a/notebooker/web/routes/report_execution.py b/notebooker/web/routes/report_execution.py index 203d82b5..d66520d8 100644 --- a/notebooker/web/routes/report_execution.py +++ b/notebooker/web/routes/report_execution.py @@ -122,6 +122,7 @@ def run_report_http(report_name): class RunReportParams(NamedTuple): report_title: AnyStr mailto: AnyStr + error_mailto: AnyStr mailfrom: AnyStr generate_pdf_output: bool hide_code: bool @@ -135,6 +136,7 @@ def validate_run_params(report_name, params, issues: List[str]) -> RunReportPara report_title = validate_title(params.get("report_title") or report_name, issues) # Get mailto email address mailto = validate_mailto(params.get("mailto"), issues) + error_mailto = validate_mailto(params.get("error_mailto"), issues) mailfrom = validate_mailto(params.get("mailfrom"), issues) # "on" comes from HTML, "True" comes from urlencoded JSON params generate_pdf_output = params.get("generate_pdf") in ("on", "True", True) @@ -144,6 +146,7 @@ def validate_run_params(report_name, params, issues: List[str]) -> RunReportPara out = RunReportParams( report_title=report_title, mailto=mailto, + error_mailto=error_mailto, mailfrom=mailfrom, generate_pdf_output=generate_pdf_output, hide_code=hide_code, @@ -165,6 +168,7 @@ def _handle_run_report( f"Handling run report with parameters report_name={report_name} " f"report_title={params.report_title}" f"mailto={params.mailto} " + f"error_mailto={params.error_mailto} " f"overrides_dict={overrides_dict} " f"generate_pdf_output={params.generate_pdf_output} " f"hide_code={params.hide_code} " @@ -180,6 +184,7 @@ def _handle_run_report( report_name=report_name, report_title=params.report_title, mailto=params.mailto, + error_mailto=params.error_mailto, overrides=overrides_dict, generate_pdf_output=params.generate_pdf_output, hide_code=params.hide_code, @@ -242,6 +247,7 @@ def _rerun_report(job_id, prepare_only=False, run_synchronously=False): result.report_name, title, result.mailto, + result.error_mailto, result.overrides, hide_code=result.hide_code, generate_pdf_output=result.generate_pdf_output, diff --git a/notebooker/web/routes/scheduling.py b/notebooker/web/routes/scheduling.py index 0cb816a1..d85e7cb1 100644 --- a/notebooker/web/routes/scheduling.py +++ b/notebooker/web/routes/scheduling.py @@ -74,6 +74,7 @@ def update_schedule(report_name): "overrides": overrides_dict, "report_title": params.report_title, "mailto": params.mailto, + "error_mailto": params.error_mailto, "mailfrom": params.mailfrom, "generate_pdf": params.generate_pdf_output, "hide_code": params.hide_code, @@ -106,6 +107,7 @@ def create_schedule(report_name): "overrides": overrides_dict, "report_title": params.report_title, "mailto": params.mailto, + "error_mailto": params.error_mailto, "mailfrom": params.mailfrom, "generate_pdf": params.generate_pdf_output, "hide_code": params.hide_code, diff --git a/notebooker/web/scheduler.py b/notebooker/web/scheduler.py index 753e9a0d..ba29c926 100644 --- a/notebooker/web/scheduler.py +++ b/notebooker/web/scheduler.py @@ -22,6 +22,7 @@ def run_report( # new parameters should be added below and be optional to avoid migrations mailfrom: Optional[str] = None, is_slideshow: bool = False, + error_mailto: Optional[str] = None, ): """ This is the entrypoint of the scheduler; APScheduler has to @@ -33,6 +34,7 @@ def run_report( report_name, report_title, mailto, + error_mailto, overrides, hide_code=hide_code, generate_pdf_output=generate_pdf, @@ -50,6 +52,7 @@ def run_report( "overrides": json.dumps(overrides), "report_title": report_title, "mailto": mailto, + "error_mailto": error_mailto, "generate_pdf": generate_pdf, "hide_code": hide_code, "scheduler_job_id": scheduler_job_id, diff --git a/notebooker/web/static/notebooker/scheduler.js b/notebooker/web/static/notebooker/scheduler.js index 749088c8..829fe4fb 100644 --- a/notebooker/web/static/notebooker/scheduler.js +++ b/notebooker/web/static/notebooker/scheduler.js @@ -139,6 +139,7 @@ function modifySchedulerModal(row) { hide_code: row.params.hide_code, generate_pdf: row.params.generate_pdf, mailto: row.params.mailto, + error_mailto: row.params.error_mailto, mailfrom: row.params.mailfrom, cronSchedule: row.cron_schedule, is_slideshow: row.params.is_slideshow, @@ -166,6 +167,7 @@ function handleAddButtonClick() { hide_code: "", generate_pdf: "", mailto: "", + error_mailto: "", mailfrom: "", cronSchedule: "", is_slideshow: "", @@ -268,6 +270,7 @@ $(document).ready(() => { overrides: formObj.overrides, cron_schedule: formObj.cronSchedule, mailto: formObj.mailto, + error_mailto: formObj.error_mailto, mailfrom: formObj.mailfrom, generate_pdf: formObj.generate_pdf, hide_code: formObj.hide_code, diff --git a/notebooker/web/templates/results.html b/notebooker/web/templates/results.html index 476a3a76..cd7808d8 100644 --- a/notebooker/web/templates/results.html +++ b/notebooker/web/templates/results.html @@ -59,6 +59,9 @@

Report Template Name:

{{ report_name }} {% if result.mailto %}

Mailing results to:

{{ result.mailto }} {% endif %} + {% if result.error_mailto %} +

Mailing errors to:

{{ result.error_mailto }} + {% endif %} {% if result.overrides %}

Parameters:

diff --git a/notebooker/web/templates/run_report.html b/notebooker/web/templates/run_report.html index 5ea1a3e3..29f54518 100644 --- a/notebooker/web/templates/run_report.html +++ b/notebooker/web/templates/run_report.html @@ -35,16 +35,23 @@

Override parameters:

-

Email to:

+

Email result to:

-

Email from (optional):

+

Email errors to:

+ +
+
+

Email from:

diff --git a/notebooker/web/templates/scheduler.html b/notebooker/web/templates/scheduler.html index 239ca862..b9b985af 100644 --- a/notebooker/web/templates/scheduler.html +++ b/notebooker/web/templates/scheduler.html @@ -57,6 +57,12 @@ +
+ +