From 777e22f720c5455fe726e618669155a5c70e0ddd Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Mon, 3 Oct 2022 15:19:47 -0700 Subject: [PATCH] chore: add 4xx error codes where applicable (#21627) (cherry picked from commit 4417c6e3e27d61d174d137c5afae4f4fb723382c) --- superset/reports/commands/exceptions.py | 18 +++++++++++++-- superset/reports/commands/execute.py | 23 ++++++++++--------- .../reports/commands_tests.py | 5 ++-- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/superset/reports/commands/exceptions.py b/superset/reports/commands/exceptions.py index 1f52aab8d8bcd..c087cf95e0d11 100644 --- a/superset/reports/commands/exceptions.py +++ b/superset/reports/commands/exceptions.py @@ -119,6 +119,7 @@ def __init__(self) -> None: class ReportScheduleInvalidError(CommandInvalidError): + status = 422 message = _("Report Schedule parameters are invalid.") @@ -135,6 +136,7 @@ class ReportScheduleUpdateFailedError(CreateFailedError): class ReportScheduleNotFoundError(CommandException): + status = 404 message = _("Report Schedule not found.") @@ -163,10 +165,12 @@ class ReportScheduleExecuteUnexpectedError(CommandException): class ReportSchedulePreviousWorkingError(CommandException): + status = 429 message = _("Report Schedule is still working, refusing to re-compute.") class ReportScheduleWorkingTimeoutError(CommandException): + status = 408 message = _("Report Schedule reached a working timeout.") @@ -188,20 +192,22 @@ class ReportScheduleCreationMethodUniquenessValidationError(CommandException): class AlertQueryMultipleRowsError(CommandException): - + status = 422 message = _("Alert query returned more than one row.") class AlertValidatorConfigError(CommandException): - + status = 422 message = _("Alert validator config error.") class AlertQueryMultipleColumnsError(CommandException): + status = 422 message = _("Alert query returned more than one column.") class AlertQueryInvalidTypeError(CommandException): + status = 422 message = _("Alert query returned a non-number value.") @@ -210,30 +216,37 @@ class AlertQueryError(CommandException): class AlertQueryTimeout(CommandException): + status = 408 message = _("A timeout occurred while executing the query.") class ReportScheduleScreenshotTimeout(CommandException): + status = 408 message = _("A timeout occurred while taking a screenshot.") class ReportScheduleCsvTimeout(CommandException): + status = 408 message = _("A timeout occurred while generating a csv.") class ReportScheduleDataFrameTimeout(CommandException): + status = 408 message = _("A timeout occurred while generating a dataframe.") class ReportScheduleAlertGracePeriodError(CommandException): + status = 429 message = _("Alert fired during grace period.") class ReportScheduleAlertEndGracePeriodError(CommandException): + status = 429 message = _("Alert ended grace period.") class ReportScheduleNotificationError(CommandException): + status = 429 message = _("Alert on grace period") @@ -250,6 +263,7 @@ class ReportScheduleUnexpectedError(CommandException): class ReportScheduleForbiddenError(ForbiddenError): + status = 403 message = _("Changing this report is forbidden") diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py index 1fbdba1c25119..52ff1cd4e75f5 100644 --- a/superset/reports/commands/execute.py +++ b/superset/reports/commands/execute.py @@ -42,7 +42,6 @@ ReportScheduleDataFrameTimeout, ReportScheduleExecuteUnexpectedError, ReportScheduleNotFoundError, - ReportScheduleNotificationError, ReportSchedulePreviousWorkingError, ReportScheduleScreenshotFailedError, ReportScheduleScreenshotTimeout, @@ -399,9 +398,9 @@ def _send( """ Sends a notification to all recipients - :raises: ReportScheduleNotificationError + :raises: NotificationError """ - notification_errors = [] + notification_errors: List[NotificationError] = [] for recipient in recipients: notification = create_notification(recipient, notification_content) try: @@ -415,15 +414,17 @@ def _send( notification.send() except NotificationError as ex: # collect notification errors but keep processing them - notification_errors.append(str(ex)) + notification_errors.append(ex) if notification_errors: - raise ReportScheduleNotificationError(";".join(notification_errors)) + # raise errors separately so that we can utilize error status codes + for error in notification_errors: + raise error def send(self) -> None: """ Creates the notification content and sends them to all recipients - :raises: ReportScheduleNotificationError + :raises: NotificationError """ notification_content = self._get_notification_content() self._send(notification_content, self._report_schedule.recipients) @@ -432,7 +433,7 @@ def send_error(self, name: str, message: str) -> None: """ Creates and sends a notification for an error, to all recipients - :raises: ReportScheduleNotificationError + :raises: NotificationError """ header_data = self._get_log_data() header_data["error_text"] = message @@ -527,7 +528,7 @@ def next(self) -> None: return self.send() self.update_report_schedule_and_log(ReportState.SUCCESS) - except CommandException as first_ex: + except Exception as first_ex: self.update_report_schedule_and_log( ReportState.ERROR, error_message=str(first_ex) ) @@ -543,7 +544,7 @@ def next(self) -> None: ReportState.ERROR, error_message=REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER, ) - except CommandException as second_ex: + except Exception as second_ex: # pylint: disable=broad-except self.update_report_schedule_and_log( ReportState.ERROR, error_message=str(second_ex) ) @@ -600,7 +601,7 @@ def next(self) -> None: if not AlertCommand(self._report_schedule).run(): self.update_report_schedule_and_log(ReportState.NOOP) return - except CommandException as ex: + except Exception as ex: self.send_error( f"Error occurred for {self._report_schedule.type}:" f" {self._report_schedule.name}", @@ -615,7 +616,7 @@ def next(self) -> None: try: self.send() self.update_report_schedule_and_log(ReportState.SUCCESS) - except CommandException as ex: + except Exception as ex: # pylint: disable=broad-except self.update_report_schedule_and_log( ReportState.ERROR, error_message=str(ex) ) diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index 12b0a19592d9d..02e5ce8bb4dbf 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -39,10 +39,10 @@ ReportScheduleCsvFailedError, ReportScheduleCsvTimeout, ReportScheduleNotFoundError, - ReportScheduleNotificationError, ReportSchedulePreviousWorkingError, ReportScheduleScreenshotFailedError, ReportScheduleScreenshotTimeout, + ReportScheduleUnexpectedError, ReportScheduleWorkingTimeoutError, ) from superset.reports.commands.execute import AsyncExecuteReportScheduleCommand @@ -113,6 +113,7 @@ def assert_log(state: str, error_message: Optional[str] = None): if state == ReportState.ERROR: # On error we send an email + print(logs) assert len(logs) == 3 else: assert len(logs) == 2 @@ -1321,7 +1322,7 @@ def test_email_dashboard_report_fails( screenshot_mock.return_value = SCREENSHOT_FILE email_mock.side_effect = SMTPException("Could not connect to SMTP XPTO") - with pytest.raises(ReportScheduleNotificationError): + with pytest.raises(ReportScheduleUnexpectedError): AsyncExecuteReportScheduleCommand( TEST_ID, create_report_email_dashboard.id, datetime.utcnow() ).run()