Skip to content

Commit

Permalink
chore: add 4xx error codes where applicable (apache#21627)
Browse files Browse the repository at this point in the history
  • Loading branch information
eschutho authored Oct 3, 2022
1 parent 84c3cf6 commit 4417c6e
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 15 deletions.
18 changes: 16 additions & 2 deletions superset/reports/commands/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def __init__(self) -> None:


class ReportScheduleInvalidError(CommandInvalidError):
status = 422
message = _("Report Schedule parameters are invalid.")


Expand All @@ -135,6 +136,7 @@ class ReportScheduleUpdateFailedError(CreateFailedError):


class ReportScheduleNotFoundError(CommandException):
status = 404
message = _("Report Schedule not found.")


Expand Down Expand Up @@ -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.")


Expand All @@ -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.")


Expand All @@ -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")


Expand All @@ -250,6 +263,7 @@ class ReportScheduleUnexpectedError(CommandException):


class ReportScheduleForbiddenError(ForbiddenError):
status = 403
message = _("Changing this report is forbidden")


Expand Down
23 changes: 12 additions & 11 deletions superset/reports/commands/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
ReportScheduleDataFrameTimeout,
ReportScheduleExecuteUnexpectedError,
ReportScheduleNotFoundError,
ReportScheduleNotificationError,
ReportSchedulePreviousWorkingError,
ReportScheduleScreenshotFailedError,
ReportScheduleScreenshotTimeout,
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
)
Expand All @@ -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)
)
Expand Down Expand Up @@ -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}",
Expand All @@ -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)
)
Expand Down
5 changes: 3 additions & 2 deletions tests/integration_tests/reports/commands_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@
ReportScheduleCsvFailedError,
ReportScheduleCsvTimeout,
ReportScheduleNotFoundError,
ReportScheduleNotificationError,
ReportSchedulePreviousWorkingError,
ReportScheduleScreenshotFailedError,
ReportScheduleScreenshotTimeout,
ReportScheduleUnexpectedError,
ReportScheduleWorkingTimeoutError,
)
from superset.reports.commands.execute import AsyncExecuteReportScheduleCommand
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 4417c6e

Please sign in to comment.