You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2022/10/07 17:14:17 UTC

[superset] 08/13: chore: add 4xx error codes where applicable (#21627)

This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 777e22f720c5455fe726e618669155a5c70e0ddd
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Mon Oct 3 15:19:47 2022 -0700

    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 ++++++++++++-----------
 tests/integration_tests/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 1f52aab8d8..c087cf95e0 100644
--- a/superset/reports/commands/exceptions.py
+++ b/superset/reports/commands/exceptions.py
@@ -119,6 +119,7 @@ class DashboardNotSavedValidationError(ValidationError):
 
 
 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 1fbdba1c25..52ff1cd4e7 100644
--- a/superset/reports/commands/execute.py
+++ b/superset/reports/commands/execute.py
@@ -42,7 +42,6 @@ from superset.reports.commands.exceptions import (
     ReportScheduleDataFrameTimeout,
     ReportScheduleExecuteUnexpectedError,
     ReportScheduleNotFoundError,
-    ReportScheduleNotificationError,
     ReportSchedulePreviousWorkingError,
     ReportScheduleScreenshotFailedError,
     ReportScheduleScreenshotTimeout,
@@ -399,9 +398,9 @@ class BaseReportState:
         """
         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 @@ class BaseReportState:
                     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 @@ class BaseReportState:
         """
         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 @@ class ReportNotTriggeredErrorState(BaseReportState):
                     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 @@ class ReportNotTriggeredErrorState(BaseReportState):
                         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 @@ class ReportSuccessState(BaseReportState):
                 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 @@ class ReportSuccessState(BaseReportState):
         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 12b0a19592..02e5ce8bb4 100644
--- a/tests/integration_tests/reports/commands_tests.py
+++ b/tests/integration_tests/reports/commands_tests.py
@@ -39,10 +39,10 @@ from superset.reports.commands.exceptions import (
     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()