You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2021/12/22 16:39:51 UTC
[superset] branch ch24621b created (now 94392a1)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch ch24621b
in repository https://gitbox.apache.org/repos/asf/superset.git.
at 94392a1 Add tests
This branch includes the following new commits:
new 5c40ab3 Update existing tests
new b1ec4f8 Add backend test
new fdfa801 feat: add force option to report screenshots
new 94392a1 Add tests
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[superset] 02/04: Add backend test
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch ch24621b
in repository https://gitbox.apache.org/repos/asf/superset.git
commit b1ec4f86592f0c802648dd028ca236081565ea4d
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Dec 9 19:00:26 2021 -0800
Add backend test
---
tests/integration_tests/reports/commands_tests.py | 37 +++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py
index 119ca6f..54629e3 100644
--- a/tests/integration_tests/reports/commands_tests.py
+++ b/tests/integration_tests/reports/commands_tests.py
@@ -722,6 +722,43 @@ def test_email_chart_alert_schedule(
@pytest.mark.usefixtures(
+ "load_birth_names_dashboard_with_slices", "create_alert_email_chart"
+)
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+def test_email_chart_alert_schedule(
+ screenshot_mock, email_mock, create_alert_email_chart,
+):
+ """
+ ExecuteReport Command: Test chart email alert schedule with screenshot
+ """
+ # setup screenshot mock
+ screenshot_mock.return_value = SCREENSHOT_FILE
+
+ with freeze_time("2020-01-01T00:00:00Z"):
+ AsyncExecuteReportScheduleCommand(
+ TEST_ID, create_alert_email_chart.id, datetime.utcnow()
+ ).run()
+
+ notification_targets = get_target_from_report_schedule(create_alert_email_chart)
+ # assert that the link sent is correct
+ assert (
+ '<a href="http://0.0.0.0:8080/superset/explore/?'
+ "form_data=%7B%22slice_id%22%3A+"
+ f"{create_alert_email_chart.chart.id}%7D&"
+ 'standalone=true&force=true">Explore in Superset</a>'
+ in email_mock.call_args[0][2]
+ )
+ # Assert the email smtp address
+ assert email_mock.call_args[0][0] == notification_targets[0]
+ # Assert the email inline screenshot
+ smtp_images = email_mock.call_args[1]["images"]
+ assert smtp_images[list(smtp_images.keys())[0]] == SCREENSHOT_FILE
+ # Assert logs are correct
+ assert_log(ReportState.SUCCESS)
+
+
+@pytest.mark.usefixtures(
"load_birth_names_dashboard_with_slices", "create_report_email_chart"
)
@patch("superset.reports.notifications.email.send_email_smtp")
[superset] 01/04: Update existing tests
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch ch24621b
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5c40ab373502e1292afd44721e3180eae47d256b
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Dec 9 18:38:07 2021 -0800
Update existing tests
---
tests/integration_tests/reports/commands_tests.py | 57 +++++++++++++++++------
1 file changed, 44 insertions(+), 13 deletions(-)
diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py
index 0e455b3..119ca6f 100644
--- a/tests/integration_tests/reports/commands_tests.py
+++ b/tests/integration_tests/reports/commands_tests.py
@@ -368,7 +368,9 @@ def create_alert_slack_chart_success():
@pytest.fixture(
- params=["alert1",]
+ params=[
+ "alert1",
+ ]
)
def create_alert_slack_chart_grace(request):
param_config = {
@@ -645,7 +647,9 @@ def create_invalid_sql_alert_email_chart(request):
@patch("superset.reports.notifications.email.send_email_smtp")
@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
def test_email_chart_report_schedule(
- screenshot_mock, email_mock, create_report_email_chart,
+ screenshot_mock,
+ email_mock,
+ create_report_email_chart,
):
"""
ExecuteReport Command: Test chart email report schedule with screenshot
@@ -684,7 +688,9 @@ def test_email_chart_report_schedule(
@patch("superset.reports.notifications.email.send_email_smtp")
@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
def test_email_chart_alert_schedule(
- screenshot_mock, email_mock, create_alert_email_chart,
+ screenshot_mock,
+ email_mock,
+ create_alert_email_chart,
):
"""
ExecuteReport Command: Test chart email alert schedule with screenshot
@@ -721,7 +727,9 @@ def test_email_chart_alert_schedule(
@patch("superset.reports.notifications.email.send_email_smtp")
@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
def test_email_chart_report_dry_run(
- screenshot_mock, email_mock, create_report_email_chart,
+ screenshot_mock,
+ email_mock,
+ create_report_email_chart,
):
"""
ExecuteReport Command: Test chart email report schedule dry run
@@ -746,7 +754,11 @@ def test_email_chart_report_dry_run(
@patch("superset.reports.notifications.email.send_email_smtp")
@patch("superset.utils.csv.get_chart_csv_data")
def test_email_chart_report_schedule_with_csv(
- csv_mock, email_mock, mock_open, mock_urlopen, create_report_email_chart_with_csv,
+ csv_mock,
+ email_mock,
+ mock_open,
+ mock_urlopen,
+ create_report_email_chart_with_csv,
):
"""
ExecuteReport Command: Test chart email report schedule with CSV
@@ -935,7 +947,9 @@ def test_email_dashboard_report_schedule(
@patch("superset.reports.notifications.slack.WebClient.files_upload")
@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
def test_slack_chart_report_schedule(
- screenshot_mock, file_upload_mock, create_report_slack_chart,
+ screenshot_mock,
+ file_upload_mock,
+ create_report_slack_chart,
):
"""
ExecuteReport Command: Test chart slack report schedule
@@ -1154,7 +1168,9 @@ def test_report_schedule_success_grace_end(
@patch("superset.reports.notifications.email.send_email_smtp")
@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
def test_alert_limit_is_applied(
- screenshot_mock, email_mock, create_alert_email_chart,
+ screenshot_mock,
+ email_mock,
+ create_alert_email_chart,
):
"""
ExecuteReport Command: Test that all alerts apply a SQL limit to stmts
@@ -1210,7 +1226,9 @@ def test_email_dashboard_report_fails(
ALERTS_ATTACH_REPORTS=True,
)
def test_slack_chart_alert(
- screenshot_mock, email_mock, create_alert_email_chart,
+ screenshot_mock,
+ email_mock,
+ create_alert_email_chart,
):
"""
ExecuteReport Command: Test chart slack alert
@@ -1267,7 +1285,9 @@ def test_slack_chart_alert_no_attachment(email_mock, create_alert_email_chart):
@patch("superset.reports.notifications.slack.WebClient")
@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
def test_slack_token_callable_chart_report(
- screenshot_mock, slack_client_mock_class, create_report_slack_chart,
+ screenshot_mock,
+ slack_client_mock_class,
+ create_report_slack_chart,
):
"""
ExecuteReport Command: Test chart slack alert (slack token callable)
@@ -1379,7 +1399,11 @@ def test_soft_timeout_screenshot(screenshot_mock, email_mock, create_alert_email
@patch("superset.reports.notifications.email.send_email_smtp")
@patch("superset.utils.csv.get_chart_csv_data")
def test_soft_timeout_csv(
- csv_mock, email_mock, mock_open, mock_urlopen, create_report_email_chart_with_csv,
+ csv_mock,
+ email_mock,
+ mock_open,
+ mock_urlopen,
+ create_report_email_chart_with_csv,
):
"""
ExecuteReport Command: Test fail on generating csv
@@ -1403,7 +1427,8 @@ def test_soft_timeout_csv(
assert email_mock.call_args[0][0] == OWNER_EMAIL
assert_log(
- ReportState.ERROR, error_message="A timeout occurred while generating a csv.",
+ ReportState.ERROR,
+ error_message="A timeout occurred while generating a csv.",
)
@@ -1415,7 +1440,11 @@ def test_soft_timeout_csv(
@patch("superset.reports.notifications.email.send_email_smtp")
@patch("superset.utils.csv.get_chart_csv_data")
def test_generate_no_csv(
- csv_mock, email_mock, mock_open, mock_urlopen, create_report_email_chart_with_csv,
+ csv_mock,
+ email_mock,
+ mock_open,
+ mock_urlopen,
+ create_report_email_chart_with_csv,
):
"""
ExecuteReport Command: Test fail on generating csv
@@ -1600,7 +1629,9 @@ def test_grace_period_error(email_mock, create_invalid_sql_alert_email_chart):
@patch("superset.reports.notifications.email.send_email_smtp")
@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
def test_grace_period_error_flap(
- screenshot_mock, email_mock, create_invalid_sql_alert_email_chart,
+ screenshot_mock,
+ email_mock,
+ create_invalid_sql_alert_email_chart,
):
"""
ExecuteReport Command: Test alert grace period on error
[superset] 03/04: feat: add force option to report screenshots
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch ch24621b
in repository https://gitbox.apache.org/repos/asf/superset.git
commit fdfa801cb6d7154053d3e5842a160b11cdc26f71
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Sat Dec 11 08:45:08 2021 -0800
feat: add force option to report screenshots
---
.../src/components/ReportModal/index.tsx | 2 +
.../src/views/CRUD/alert/AlertReportModal.tsx | 2 +
...aa3ff_add_force_screenshot_to_alerts_reports.py | 61 ++++++++++++++++++++++
superset/models/reports.py | 4 ++
superset/reports/commands/execute.py | 8 +--
superset/reports/schemas.py | 2 +
6 files changed, 72 insertions(+), 7 deletions(-)
diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx
index 178e394..fc5469d 100644
--- a/superset-frontend/src/components/ReportModal/index.tsx
+++ b/superset-frontend/src/components/ReportModal/index.tsx
@@ -71,6 +71,7 @@ export interface ReportObject {
validator_type: string;
working_timeout: number;
creation_method: string;
+ force_screenshot: boolean;
}
interface ChartObject {
@@ -227,6 +228,7 @@ const ReportModal: FunctionComponent<ReportProps> = ({
active: true,
report_format: currentReport?.report_format || defaultNotificationFormat,
timezone: currentReport?.timezone,
+ force_screenshot: false,
};
if (isEditMode) {
diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx
index 5dfeeec..506a2c9 100644
--- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx
+++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx
@@ -154,6 +154,7 @@ const DEFAULT_ALERT = {
sql: '',
validator_config_json: {},
validator_type: '',
+ force_screenshot: true,
grace_period: undefined,
};
@@ -512,6 +513,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
const data: any = {
...currentAlert,
type: isReport ? 'Report' : 'Alert',
+ force_screenshot: isReport ? 'false' : 'true',
validator_type: conditionNotNull ? 'not null' : 'operator',
validator_config_json: conditionNotNull
? {}
diff --git a/superset/migrations/versions/bb38f40aa3ff_add_force_screenshot_to_alerts_reports.py b/superset/migrations/versions/bb38f40aa3ff_add_force_screenshot_to_alerts_reports.py
new file mode 100644
index 0000000..57913fe
--- /dev/null
+++ b/superset/migrations/versions/bb38f40aa3ff_add_force_screenshot_to_alerts_reports.py
@@ -0,0 +1,61 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Add force_screenshot to alerts/reports
+
+Revision ID: bb38f40aa3ff
+Revises: abe27eaf93db
+Create Date: 2021-12-10 19:25:29.802949
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "bb38f40aa3ff"
+down_revision = "abe27eaf93db"
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy.ext.declarative import declarative_base
+
+from superset import db
+
+Base = declarative_base()
+
+
+class ReportSchedule(Base):
+ __tablename__ = "report_schedule"
+
+ id = sa.Column(sa.Integer, primary_key=True)
+ type = sa.Column(sa.String(50), nullable=False)
+ force_screenshot = sa.Column(sa.Boolean, default=False)
+
+
+def upgrade():
+ with op.batch_alter_table("report_schedule") as batch_op:
+ batch_op.add_column(sa.Column("force_screenshot", sa.Boolean(), default=False))
+
+ bind = op.get_bind()
+ session = db.Session(bind=bind)
+
+ for report in session.query(ReportSchedule).all():
+ report.force_screenshot = report.type == "Alert"
+
+ session.commit()
+
+
+def downgrade():
+ with op.batch_alter_table("report_schedule") as batch_op:
+ batch_op.drop_column("force_screenshot")
diff --git a/superset/models/reports.py b/superset/models/reports.py
index a0dc599..1ecf698 100644
--- a/superset/models/reports.py
+++ b/superset/models/reports.py
@@ -148,6 +148,10 @@ class ReportSchedule(Model, AuditMixinNullable):
# Store the selected dashboard tabs etc.
extra = Column(Text, default="{}")
+ # (Reports) When generating a screenshot, bypass the cache? This is always true for
+ # Alerts.
+ force_screenshot = Column(Boolean, default=False)
+
def __repr__(self) -> str:
return str(self.name)
diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py
index 2789bb2..2574ed5 100644
--- a/superset/reports/commands/execute.py
+++ b/superset/reports/commands/execute.py
@@ -148,13 +148,7 @@ class BaseReportState:
"""
# For alerts we always want to send a fresh screenshot, bypassing
# the cache.
- # TODO (betodealmeida): allow to specify per report if users want
- # to bypass the cache as well.
- force = (
- "true"
- if self._report_schedule.type == ReportScheduleType.ALERT
- else "false"
- )
+ force = "true" if self._report_schedule.force_screenshot else "false"
if self._report_schedule.chart:
if result_format in {
diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py
index 3f2fb44..f4c8548 100644
--- a/superset/reports/schemas.py
+++ b/superset/reports/schemas.py
@@ -202,6 +202,7 @@ class ReportSchedulePostSchema(Schema):
default=ReportDataFormat.VISUALIZATION,
validate=validate.OneOf(choices=tuple(key.value for key in ReportDataFormat)),
)
+ force_screenshot = fields.Boolean(default=False)
@validates_schema
def validate_report_references( # pylint: disable=unused-argument,no-self-use
@@ -292,3 +293,4 @@ class ReportSchedulePutSchema(Schema):
default=ReportDataFormat.VISUALIZATION,
validate=validate.OneOf(choices=tuple(key.value for key in ReportDataFormat)),
)
+ force_screenshot = fields.Boolean(default=False)
[superset] 04/04: Add tests
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch ch24621b
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 94392a17ab7093e69f07b61d6c73942168f4c86a
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Sun Dec 12 10:16:33 2021 -0800
Add tests
---
superset/db_engine_specs/postgres.py | 12 ++++-
tests/integration_tests/reports/commands_tests.py | 58 +++++++++++++++++++++++
tests/integration_tests/reports/utils.py | 2 +
3 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/superset/db_engine_specs/postgres.py b/superset/db_engine_specs/postgres.py
index c2951c3..73da187 100644
--- a/superset/db_engine_specs/postgres.py
+++ b/superset/db_engine_specs/postgres.py
@@ -195,8 +195,16 @@ class PostgresEngineSpec(PostgresBaseEngineSpec, BasicParametersMixin):
lambda match: ARRAY(int(match[2])) if match[2] else String(),
utils.GenericDataType.STRING,
),
- (re.compile(r"^json.*", re.IGNORECASE), JSON(), utils.GenericDataType.STRING,),
- (re.compile(r"^enum.*", re.IGNORECASE), ENUM(), utils.GenericDataType.STRING,),
+ (
+ re.compile(r"^json.*", re.IGNORECASE),
+ JSON(),
+ utils.GenericDataType.STRING,
+ ),
+ (
+ re.compile(r"^enum.*", re.IGNORECASE),
+ ENUM(),
+ utils.GenericDataType.STRING,
+ ),
)
@classmethod
diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py
index 54629e3..cd730cd 100644
--- a/tests/integration_tests/reports/commands_tests.py
+++ b/tests/integration_tests/reports/commands_tests.py
@@ -135,6 +135,7 @@ def create_report_notification(
grace_period: Optional[int] = None,
report_format: Optional[ReportDataFormat] = None,
name: Optional[str] = None,
+ force_screenshot: bool = False,
) -> ReportSchedule:
report_type = report_type or ReportScheduleType.REPORT
target = email_target or slack_channel
@@ -174,6 +175,7 @@ def create_report_notification(
validator_config_json=validator_config_json,
grace_period=grace_period,
report_format=report_format or ReportDataFormat.VISUALIZATION,
+ force_screenshot=force_screenshot,
)
return report_schedule
@@ -219,6 +221,18 @@ def create_report_email_chart():
@pytest.fixture()
+def create_report_email_chart_force_screenshot():
+ with app.app_context():
+ chart = db.session.query(Slice).first()
+ report_schedule = create_report_notification(
+ email_target="target@email.com", chart=chart, force_screenshot=True
+ )
+ yield report_schedule
+
+ cleanup_report_schedule(report_schedule)
+
+
+@pytest.fixture()
def create_report_email_chart_with_csv():
with app.app_context():
chart = db.session.query(Slice).first()
@@ -482,6 +496,7 @@ def create_alert_email_chart(request):
validator_config_json=param_config[request.param][
"validator_config_json"
],
+ force_screenshot=True,
)
yield report_schedule
@@ -683,6 +698,49 @@ def test_email_chart_report_schedule(
@pytest.mark.usefixtures(
+ "load_birth_names_dashboard_with_slices",
+ "create_report_email_chart_force_screenshot",
+)
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+def test_email_chart_report_schedule_force_screenshot(
+ screenshot_mock, email_mock, create_report_email_chart_force_screenshot,
+):
+ """
+ ExecuteReport Command: Test chart email report schedule with screenshot
+
+ In this test ``force_screenshot`` is true, and the screenshot URL should
+ reflect that.
+ """
+ # setup screenshot mock
+ screenshot_mock.return_value = SCREENSHOT_FILE
+
+ with freeze_time("2020-01-01T00:00:00Z"):
+ AsyncExecuteReportScheduleCommand(
+ TEST_ID, create_report_email_chart_force_screenshot.id, datetime.utcnow()
+ ).run()
+
+ notification_targets = get_target_from_report_schedule(
+ create_report_email_chart_force_screenshot
+ )
+ # assert that the link sent is correct
+ assert (
+ '<a href="http://0.0.0.0:8080/superset/explore/?'
+ "form_data=%7B%22slice_id%22%3A+"
+ f"{create_report_email_chart_force_screenshot.chart.id}%7D&"
+ 'standalone=true&force=true">Explore in Superset</a>'
+ in email_mock.call_args[0][2]
+ )
+ # Assert the email smtp address
+ assert email_mock.call_args[0][0] == notification_targets[0]
+ # Assert the email inline screenshot
+ smtp_images = email_mock.call_args[1]["images"]
+ assert smtp_images[list(smtp_images.keys())[0]] == SCREENSHOT_FILE
+ # Assert logs are correct
+ assert_log(ReportState.SUCCESS)
+
+
+@pytest.mark.usefixtures(
"load_birth_names_dashboard_with_slices", "create_alert_email_chart"
)
@patch("superset.reports.notifications.email.send_email_smtp")
diff --git a/tests/integration_tests/reports/utils.py b/tests/integration_tests/reports/utils.py
index 6cf55a1..2adf9cc 100644
--- a/tests/integration_tests/reports/utils.py
+++ b/tests/integration_tests/reports/utils.py
@@ -51,6 +51,7 @@ def insert_report_schedule(
recipients: Optional[List[ReportRecipients]] = None,
report_format: Optional[ReportDataFormat] = None,
logs: Optional[List[ReportExecutionLog]] = None,
+ force_screenshot: bool = False,
) -> ReportSchedule:
owners = owners or []
recipients = recipients or []
@@ -75,6 +76,7 @@ def insert_report_schedule(
logs=logs,
last_state=last_state,
report_format=report_format,
+ force_screenshot=force_screenshot,
)
db.session.add(report_schedule)
db.session.commit()