Skip to content

Commit

Permalink
better error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
eschutho committed Nov 10, 2022
1 parent 46f7c67 commit baa3c26
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 12 deletions.
24 changes: 16 additions & 8 deletions superset/reports/commands/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
CreateDashboardPermalinkCommand,
)
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetException
from superset.exceptions import SupersetErrorsException, SupersetException
from superset.extensions import feature_flag_manager, machine_auth_provider_factory
from superset.reports.commands.alert import AlertCommand
from superset.reports.commands.exceptions import (
Expand Down Expand Up @@ -106,7 +106,6 @@ def update_report_schedule_and_log(
Update the report schedule state et al. and reflect the change in the execution
log.
"""

self.update_report_schedule(state)
self.create_log(error_message)

Expand Down Expand Up @@ -534,25 +533,34 @@ def next(self) -> None:
return
self.send()
self.update_report_schedule_and_log(ReportState.SUCCESS)
except Exception as first_ex:
except (SupersetErrorsException, Exception) as first_ex:
error_message = str(first_ex)
if isinstance(first_ex, SupersetErrorsException):
error_message = ";".join([error.message for error in first_ex.errors])

self.update_report_schedule_and_log(
ReportState.ERROR, error_message=str(first_ex)
ReportState.ERROR, error_message=error_message
)

# TODO (dpgaspar) convert this logic to a new state eg: ERROR_ON_GRACE
if not self.is_in_error_grace_period():
second_error_message = REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER
try:
self.send_error(
f"Error occurred for {self._report_schedule.type}:"
f" {self._report_schedule.name}",
str(first_ex),
)
self.update_report_schedule_and_log(
ReportState.ERROR,
error_message=REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER,

except SupersetErrorsException as second_ex:
second_error_message = ";".join(
[error.message for error in second_ex.errors]
)
except Exception as second_ex: # pylint: disable=broad-except
second_error_message = str(second_ex)
finally:
self.update_report_schedule_and_log(
ReportState.ERROR, error_message=str(second_ex)
ReportState.ERROR, error_message=second_error_message
)
raise first_ex

Expand Down
5 changes: 5 additions & 0 deletions superset/reports/notifications/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from flask_babel import gettext as __

from superset import app
from superset.exceptions import SupersetErrorsException
from superset.reports.models import ReportRecipientType
from superset.reports.notifications.base import BaseNotification
from superset.reports.notifications.exceptions import NotificationError
Expand Down Expand Up @@ -210,5 +211,9 @@ def send(self) -> None:
logger.info(
"Report sent to email, notification content is %s", content.header_data
)
except SupersetErrorsException as ex:
raise NotificationError(
";".join([error.message for error in ex.errors])
) from ex
except Exception as ex:
raise NotificationError(str(ex)) from ex
3 changes: 2 additions & 1 deletion superset/tasks/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from superset.reports.dao import ReportScheduleDAO
from superset.tasks.cron_util import cron_schedule_window
from superset.utils.celery import session_scope
from superset.utils.core import LoggerLevel
from superset.utils.log import get_logger_from_status

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -98,7 +99,7 @@ def execute(self: Celery.task, report_schedule_id: int, scheduled_dttm: str) ->
logger_func(
"A downstream %s occurred while generating a report: %s", level, task_id
)
if level == "exception":
if level == LoggerLevel.EXCEPTION:
self.update_state(state="FAILURE")


Expand Down
6 changes: 6 additions & 0 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ class DatasourceType(str, Enum):
VIEW = "view"


class LoggerLevel(str, Enum):
INFO = "info"
WARNING = "warning"
EXCEPTION = "exception"


class HeaderDataType(TypedDict):
notification_format: str
owners: List[int]
Expand Down
9 changes: 7 additions & 2 deletions superset/utils/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
from sqlalchemy.exc import SQLAlchemyError
from typing_extensions import Literal

from superset.utils.core import get_user_id
from superset.utils.core import get_user_id, LoggerLevel

if TYPE_CHECKING:
from superset.stats_logger import BaseStatsLogger
Expand Down Expand Up @@ -85,7 +85,12 @@ def get_logger_from_status(
Return logger method by status of exception.
Maps logger level to status code level
"""
log_map = {"2": "info", "3": "info", "4": "warning", "5": "exception"}
log_map = {
"2": LoggerLevel.INFO,
"3": LoggerLevel.INFO,
"4": LoggerLevel.WARNING,
"5": LoggerLevel.EXCEPTION,
}
log_level = log_map[str(status)[0]]

return (getattr(logger, log_level), log_level)
Expand Down
1 change: 0 additions & 1 deletion tests/integration_tests/event_logger_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
AbstractEventLogger,
DBEventLogger,
get_event_logger_from_cfg_value,
get_logger_from_status,
)
from tests.integration_tests.test_app import app

Expand Down

0 comments on commit baa3c26

Please sign in to comment.