From 153b60a7670596beaa29be87eb62c8ba10b1baa9 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 28 Jan 2021 14:08:06 +0000 Subject: [PATCH 1/4] fix(reports): handle exceptions properly in scope --- superset/tasks/scheduler.py | 6 ++++-- superset/utils/celery.py | 7 ++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/superset/tasks/scheduler.py b/superset/tasks/scheduler.py index 1b0d6b523aecc..c7e5d09acf82c 100644 --- a/superset/tasks/scheduler.py +++ b/superset/tasks/scheduler.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. import logging +from dateutil import parser from datetime import datetime, timedelta from typing import Iterator @@ -60,9 +61,10 @@ def scheduler() -> None: @celery_app.task(name="reports.execute") -def execute(report_schedule_id: int, scheduled_dttm: datetime) -> None: +def execute(report_schedule_id: int, scheduled_dttm: str) -> None: try: - AsyncExecuteReportScheduleCommand(report_schedule_id, scheduled_dttm).run() + scheduled_dttm_ = parser.parse(scheduled_dttm) + AsyncExecuteReportScheduleCommand(report_schedule_id, scheduled_dttm_).run() except ReportScheduleUnexpectedError as ex: logger.error("An unexpected occurred while executing the report: %s", ex) except CommandException as ex: diff --git a/superset/utils/celery.py b/superset/utils/celery.py index bf8cfc2a76e00..c39f9b2d705cb 100644 --- a/superset/utils/celery.py +++ b/superset/utils/celery.py @@ -17,7 +17,8 @@ import logging from typing import Iterator -import sqlalchemy as sa +from sqlalchemy import create_engine +from sqlalchemy.exc import SQLAlchemyError from contextlib2 import contextmanager from sqlalchemy.orm import Session, sessionmaker from sqlalchemy.pool import NullPool @@ -38,7 +39,7 @@ def session_scope(nullpool: bool) -> Iterator[Session]: in a future version of Superset." ) if nullpool: - engine = sa.create_engine(database_uri, poolclass=NullPool) + engine = create_engine(database_uri, poolclass=NullPool) session_class = sessionmaker() session_class.configure(bind=engine) session = session_class() @@ -49,7 +50,7 @@ def session_scope(nullpool: bool) -> Iterator[Session]: try: yield session session.commit() - except Exception as ex: + except SQLAlchemyError as ex: session.rollback() logger.exception(ex) raise From bb576daf9201f9d2cc2523d4202483d63af6cbb0 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 28 Jan 2021 15:30:08 +0000 Subject: [PATCH 2/4] list and test commit removal --- superset/tasks/scheduler.py | 2 +- tests/reports/commands_tests.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset/tasks/scheduler.py b/superset/tasks/scheduler.py index c7e5d09acf82c..878812b0f6734 100644 --- a/superset/tasks/scheduler.py +++ b/superset/tasks/scheduler.py @@ -15,11 +15,11 @@ # specific language governing permissions and limitations # under the License. import logging -from dateutil import parser from datetime import datetime, timedelta from typing import Iterator import croniter +from dateutil import parser from superset import app from superset.commands.exceptions import CommandException diff --git a/tests/reports/commands_tests.py b/tests/reports/commands_tests.py index dac34374da50b..998563574c497 100644 --- a/tests/reports/commands_tests.py +++ b/tests/reports/commands_tests.py @@ -62,7 +62,7 @@ def get_target_from_report_schedule(report_schedule) -> List[str]: def assert_log(state: str, error_message: Optional[str] = None): - db.session.commit() + # db.session.commit() logs = db.session.query(ReportExecutionLog).all() if state == ReportState.WORKING: assert len(logs) == 1 @@ -574,7 +574,7 @@ def test_report_schedule_working_timeout(create_report_slack_chart_working): ).run() # Only needed for MySQL, understand why - db.session.commit() + # db.session.commit() logs = db.session.query(ReportExecutionLog).all() assert len(logs) == 1 assert logs[0].error_message == ReportScheduleWorkingTimeoutError.message @@ -598,7 +598,7 @@ def test_report_schedule_success_grace(create_alert_slack_chart_success): create_alert_slack_chart_success.id, datetime.utcnow() ).run() - db.session.commit() + # db.session.commit() assert create_alert_slack_chart_success.last_state == ReportState.GRACE @@ -617,7 +617,7 @@ def test_report_schedule_success_grace_end(create_alert_slack_chart_grace): create_alert_slack_chart_grace.id, datetime.utcnow() ).run() - db.session.commit() + # db.session.commit() assert create_alert_slack_chart_grace.last_state == ReportState.NOOP From 0c9aeda7eb88453c82b4163b094237a71e78d426 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 28 Jan 2021 16:19:14 +0000 Subject: [PATCH 3/4] revert removing commits from tests --- tests/reports/commands_tests.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/reports/commands_tests.py b/tests/reports/commands_tests.py index 998563574c497..dac34374da50b 100644 --- a/tests/reports/commands_tests.py +++ b/tests/reports/commands_tests.py @@ -62,7 +62,7 @@ def get_target_from_report_schedule(report_schedule) -> List[str]: def assert_log(state: str, error_message: Optional[str] = None): - # db.session.commit() + db.session.commit() logs = db.session.query(ReportExecutionLog).all() if state == ReportState.WORKING: assert len(logs) == 1 @@ -574,7 +574,7 @@ def test_report_schedule_working_timeout(create_report_slack_chart_working): ).run() # Only needed for MySQL, understand why - # db.session.commit() + db.session.commit() logs = db.session.query(ReportExecutionLog).all() assert len(logs) == 1 assert logs[0].error_message == ReportScheduleWorkingTimeoutError.message @@ -598,7 +598,7 @@ def test_report_schedule_success_grace(create_alert_slack_chart_success): create_alert_slack_chart_success.id, datetime.utcnow() ).run() - # db.session.commit() + db.session.commit() assert create_alert_slack_chart_success.last_state == ReportState.GRACE @@ -617,7 +617,7 @@ def test_report_schedule_success_grace_end(create_alert_slack_chart_grace): create_alert_slack_chart_grace.id, datetime.utcnow() ).run() - # db.session.commit() + db.session.commit() assert create_alert_slack_chart_grace.last_state == ReportState.NOOP From 8db9de088b2530642352dfea1df0bc99194a06ca Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 28 Jan 2021 16:47:30 +0000 Subject: [PATCH 4/4] lint --- superset/utils/celery.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/utils/celery.py b/superset/utils/celery.py index c39f9b2d705cb..be543a0c2ce8b 100644 --- a/superset/utils/celery.py +++ b/superset/utils/celery.py @@ -17,9 +17,9 @@ import logging from typing import Iterator +from contextlib2 import contextmanager from sqlalchemy import create_engine from sqlalchemy.exc import SQLAlchemyError -from contextlib2 import contextmanager from sqlalchemy.orm import Session, sessionmaker from sqlalchemy.pool import NullPool