From 91fd1acbe19148b2668c3339615941afaf9e45b9 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 15:41:48 +0000 Subject: [PATCH 01/26] Added S3 as features --- docker/docker-frontend.sh | 1 + docker/pythonpath_dev/superset_config.py | 7 + ...0_added_aws_s3_columns_to_reports_model.py | 53 +++++++ superset/reports/models.py | 7 +- superset/reports/notifications/s3.py | 131 ++++++++++++++++++ superset/views/base.py | 23 +-- 6 files changed, 211 insertions(+), 11 deletions(-) create mode 100755 superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py create mode 100644 superset/reports/notifications/s3.py diff --git a/docker/docker-frontend.sh b/docker/docker-frontend.sh index 85c57cbf0fc88..644dee9f50a84 100755 --- a/docker/docker-frontend.sh +++ b/docker/docker-frontend.sh @@ -19,6 +19,7 @@ set -e # Packages needed for puppeteer: apt update +chmod -R 777 /root if [ "$PUPPETEER_SKIP_CHROMIUM_DOWNLOAD" = "false" ]; then apt install -y chromium fi diff --git a/docker/pythonpath_dev/superset_config.py b/docker/pythonpath_dev/superset_config.py index 005cc600ae1bf..196c7ea42a156 100644 --- a/docker/pythonpath_dev/superset_config.py +++ b/docker/pythonpath_dev/superset_config.py @@ -100,6 +100,13 @@ class CeleryConfig: SQLLAB_CTAS_NO_LIMIT = True +# AWS S3 reporting - uncomment to use +FEATURE_FLAGS = {"ALERT_REPORTS": True, "ENABLE_AWS": True} +ALERT_REPORTS_NOTIFICATION_DRY_RUN = False +# AWS Credentials +AWS_ACCESS_KEY = '####' +AWS_SECRET_KEY = '####' + # # Optionally import superset_config_docker.py (which will have been included on # the PYTHONPATH) in order to allow for local settings to be overridden diff --git a/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py b/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py new file mode 100755 index 0000000000000..5b433ebf87563 --- /dev/null +++ b/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py @@ -0,0 +1,53 @@ +# 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. +"""Added AWS S3 columns to reports model + +Revision ID: 4925b0889720 +Revises: be1b217cd8cd +Create Date: 2024-03-18 15:23:10.575512 + +""" + +# revision identifiers, used by Alembic. +revision = '4925b0889720' +down_revision = 'be1b217cd8cd' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql +from sqlalchemy_utils import EncryptedType + +def upgrade(): + + op.add_column( + "report_schedule", + sa.Column("aws_key", EncryptedType(sa.String(1024)), nullable=True), + ) + op.add_column( + "report_schedule", + sa.Column("aws_secret_key", EncryptedType(sa.String(1024)), nullable=True), + ) + op.add_column( + "report_schedule", + sa.Column("aws_s3_types", sa.String(length=200), nullable=True), + ) + + +def downgrade(): + op.drop_column("report_schedule", "aws_key") + op.drop_column("report_schedule", "aws_secret_key") + op.drop_column("report_schedule", "aws_s3_types") \ No newline at end of file diff --git a/superset/reports/models.py b/superset/reports/models.py index 022db5dc6fce6..1c4f3c191ac8a 100644 --- a/superset/reports/models.py +++ b/superset/reports/models.py @@ -34,7 +34,7 @@ from sqlalchemy.schema import UniqueConstraint from sqlalchemy_utils import UUIDType -from superset.extensions import security_manager +from superset.extensions import encrypted_field_factory, security_manager from superset.models.core import Database from superset.models.dashboard import Dashboard from superset.models.helpers import AuditMixinNullable, ExtraJSONMixin @@ -61,6 +61,7 @@ class ReportScheduleValidatorType(StrEnum): class ReportRecipientType(StrEnum): EMAIL = "Email" SLACK = "Slack" + S3 = "S3" class ReportState(StrEnum): @@ -168,7 +169,9 @@ class ReportSchedule(AuditMixinNullable, ExtraJSONMixin, Model): custom_width = Column(Integer, nullable=True) custom_height = Column(Integer, nullable=True) - + aws_key = Column(encrypted_field_factory.create(String(1024))) + aws_secret_key = Column(encrypted_field_factory.create(String(1024))) + aws_s3_types = Column(String(200)) extra: ReportScheduleExtra # type: ignore def __repr__(self) -> str: diff --git a/superset/reports/notifications/s3.py b/superset/reports/notifications/s3.py new file mode 100644 index 0000000000000..6368697c750df --- /dev/null +++ b/superset/reports/notifications/s3.py @@ -0,0 +1,131 @@ +import datetime +import json +import logging +from io import BytesIO +from uuid import uuid4 + +import boto3 + +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 + +logger = logging.getLogger(__name__) + + +class S3SubTypes: # pylint: disable=too-few-public-methods + """ + Defines different types of AWS S3 configurations. + """ + + S3_CRED = "AWS_S3_credentials" + S3_CONFIG = "AWS_S3_pyconfig" + S3_ROLE = "AWS_S3_IAM" + + +class S3Notification(BaseNotification): # pylint: disable=too-few-public-methods + # pylint: disable= too-many-arguments, invalid-name + type = ReportRecipientType.S3 + + def _get_inline_files(self): + current_datetime = datetime.datetime.now() + formatted_date = current_datetime.strftime("%Y-%m-%d") + report_name = self._content.name + name_prefix = f"{report_name}/{formatted_date}/" + + if self._content.csv: + data = { + f"{name_prefix}{report_name}-{str(uuid4())[:8]}.csv": self._content.csv + } + return data + if self._content.screenshots: + images = { + f"{name_prefix}Screenshot-{str(uuid4())[:8]}.png": screenshot + for screenshot in self._content.screenshots + } + return images + return [] + + def _execute_s3_upload( + self, + file_body, + bucket_name, + contentType, + aws_access_key_id=None, + aws_secret_access_key=None, + ): + for key, file in file_body.items(): + file = BytesIO(file) + s3 = boto3.client( + "s3", + aws_access_key_id=aws_access_key_id, + aws_secret_access_key=aws_secret_access_key, + ) + s3.upload_fileobj( + file, + bucket_name, + key, + ExtraArgs={ + "Metadata": {"Content-Disposition": "inline"}, + "ContentType": contentType, + }, + ) + + logger.info( + "Report sent to Aws S3 Bucket, notification content is %s", + self._content.header_data, + ) + + def send(self): + files = self._get_inline_files() + file_type = "csv" if self._content.csv else "png" + bucket_name = json.loads(self._recipient.recipient_config_json)["target"] + s3_Subtype = self._awsConfiguration.aws_S3_types + + try: + + if s3_Subtype == S3SubTypes.S3_CRED: + + aws_access_key_id = self._awsConfiguration.aws_key + aws_secret_access_key = self._awsConfiguration.aws_secretKey + + self._execute_s3_upload( + file_body=files, + bucket_name=bucket_name, + contentType=file_type, + aws_access_key_id=aws_access_key_id, + aws_secret_access_key=aws_secret_access_key, + ) + + elif s3_Subtype == S3SubTypes.S3_ROLE: + self._execute_s3_upload( + file_body=files, bucket_name=bucket_name, contentType=file_type + ) + + elif s3_Subtype == S3SubTypes.S3_CONFIG: + + aws_access_key_id = app.config["AWS_ACCESS_KEY"] + aws_secret_access_key = app.config["AWS_SECRET_KEY"] + + self._execute_s3_upload( + file_body=files, + bucket_name=bucket_name, + contentType=file_type, + aws_access_key_id=aws_access_key_id, + aws_secret_access_key=aws_secret_access_key, + ) + else: + msg = ( + f"Unsupported AWS S3 method, Must be {S3SubTypes.S3_CONFIG} | " + f"{S3SubTypes.S3_CRED} | {S3SubTypes.S3_ROLE}" + ) + + logger.error(msg) + 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 \ No newline at end of file diff --git a/superset/views/base.py b/superset/views/base.py index c8b48627103d7..145b996396c58 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -391,16 +391,21 @@ def cached_common_bootstrap_data( # pylint: disable=unused-argument for k in FRONTEND_CONF_KEYS } - if conf.get("SLACK_API_TOKEN"): - frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"] = [ - ReportRecipientType.EMAIL, - ReportRecipientType.SLACK, - ] - else: - frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"] = [ - ReportRecipientType.EMAIL, - ] + isAwsConfigured = ( + get_feature_flags()["ENABLE_AWS"] + if "ENABLE_AWS" in get_feature_flags() + else False + ) + frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"] = [ReportRecipientType.EMAIL] + if conf.get("SLACK_API_TOKEN"): + frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"].append( + ReportRecipientType.SLACK + ) + if isAwsConfigured: + frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"].append( + ReportRecipientType.S3 + ) # verify client has google sheets installed available_specs = get_available_engine_specs() frontend_config["HAS_GSHEETS_INSTALLED"] = bool(available_specs[GSheetsEngineSpec]) From 952cf2fe2a7205f64625bd1100a224fc1283282a Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 16:33:41 +0000 Subject: [PATCH 02/26] Updated the all required changes till what we did on 2.1.0 --- superset/commands/report/execute.py | 20 ++++++++++++++++-- superset/reports/api.py | 6 ++++++ superset/reports/notifications/__init__.py | 13 +++++++++--- superset/reports/notifications/base.py | 12 ++++++++++- superset/reports/notifications/s3.py | 6 +++--- superset/reports/schemas.py | 24 ++++++++++++++++++++++ 6 files changed, 72 insertions(+), 9 deletions(-) diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py index 5cacb66134a5d..b8aa6a8bb3653 100644 --- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -64,7 +64,7 @@ ReportState, ) from superset.reports.notifications import create_notification -from superset.reports.notifications.base import NotificationContent +from superset.reports.notifications.base import AwsConfiguration, NotificationContent from superset.reports.notifications.exceptions import NotificationError from superset.tasks.utils import get_executor from superset.utils.core import HeaderDataType, override_user @@ -395,6 +395,15 @@ def _get_notification_content(self) -> NotificationContent: embedded_data=embedded_data, header_data=header_data, ) + def _get_aws_configuration(self) -> AwsConfiguration: + # pylint: disable=invalid-name + aws_key = self._report_schedule.aws_key + aws_secret_key = self._report_schedule.aws_secret_key + aws_s3_types = self._report_schedule.aws_s3_types + + return AwsConfiguration( + aws_key=aws_key, aws_secretKey=aws_secret_key, aws_S3_types=aws_s3_types + ) def _send( self, @@ -408,7 +417,14 @@ def _send( """ notification_errors: list[SupersetError] = [] for recipient in recipients: - notification = create_notification(recipient, notification_content) + if recipient.type == ReportRecipientType.S3: + # pylint: disable=invalid-name + aws_configuration = self._get_aws_configuration() + notification = create_notification( + recipient, notification_content, aws_configuration + ) + else: + notification = create_notification(recipient, notification_content) try: if app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"]: logger.info( diff --git a/superset/reports/api.py b/superset/reports/api.py index 8238213fefd72..313a1239c0289 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -119,6 +119,9 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]: "validator_config_json", "validator_type", "working_timeout", + "aws_secret_key", + "aws_key", + "aws_s3_types" ] show_select_columns = show_columns + [ "chart.datasource_id", @@ -151,6 +154,9 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]: "recipients.type", "timezone", "type", + "aws_secret_key", + "aws_key", + "aws_s3_types" ] add_columns = [ "active", diff --git a/superset/reports/notifications/__init__.py b/superset/reports/notifications/__init__.py index b9fff48cf7c64..19f930e98713e 100644 --- a/superset/reports/notifications/__init__.py +++ b/superset/reports/notifications/__init__.py @@ -15,13 +15,20 @@ # specific language governing permissions and limitations # under the License. from superset.reports.models import ReportRecipients -from superset.reports.notifications.base import BaseNotification, NotificationContent +from superset.reports.notifications.base import ( + AwsConfiguration, + BaseNotification, + NotificationContent, +) from superset.reports.notifications.email import EmailNotification +from superset.reports.notifications.s3 import S3Notification from superset.reports.notifications.slack import SlackNotification def create_notification( - recipient: ReportRecipients, notification_content: NotificationContent + recipient: ReportRecipients, + notification_content: NotificationContent, + aws_configuration: AwsConfiguration = None ) -> BaseNotification: """ Notification polymorphic factory @@ -29,7 +36,7 @@ def create_notification( """ for plugin in BaseNotification.plugins: if plugin.type == recipient.type: - return plugin(recipient, notification_content) + return plugin(recipient, notification_content, aws_configuration) raise Exception( # pylint: disable=broad-exception-raised "Recipient type not supported" ) diff --git a/superset/reports/notifications/base.py b/superset/reports/notifications/base.py index 640b326fc53d8..0121860f4eec8 100644 --- a/superset/reports/notifications/base.py +++ b/superset/reports/notifications/base.py @@ -34,6 +34,13 @@ class NotificationContent: url: Optional[str] = None # url to chart/dashboard for this screenshot embedded_data: Optional[pd.DataFrame] = None +@dataclass +class AwsConfiguration: + # pylint: disable=invalid-name + aws_key: Optional[str] = None + aws_secret_key: Optional[str] = None + aws_s3_types: Optional[str] = None + class BaseNotification: # pylint: disable=too-few-public-methods """ @@ -55,10 +62,13 @@ def __init_subclass__(cls, *args: Any, **kwargs: Any) -> None: cls.plugins.append(cls) def __init__( - self, recipient: ReportRecipients, content: NotificationContent + self, recipient: ReportRecipients, + content: NotificationContent, + aws_configuration: AwsConfiguration = None ) -> None: self._recipient = recipient self._content = content + self._aws_configuration = aws_configuration def send(self) -> None: raise NotImplementedError() diff --git a/superset/reports/notifications/s3.py b/superset/reports/notifications/s3.py index 6368697c750df..71cf4c1f3e66b 100644 --- a/superset/reports/notifications/s3.py +++ b/superset/reports/notifications/s3.py @@ -82,14 +82,14 @@ def send(self): files = self._get_inline_files() file_type = "csv" if self._content.csv else "png" bucket_name = json.loads(self._recipient.recipient_config_json)["target"] - s3_Subtype = self._awsConfiguration.aws_S3_types + s3_Subtype = self._aws_configuration.aws_S3_types try: if s3_Subtype == S3SubTypes.S3_CRED: - aws_access_key_id = self._awsConfiguration.aws_key - aws_secret_access_key = self._awsConfiguration.aws_secretKey + aws_access_key_id = self._aws_configuration.aws_key + aws_secret_access_key = self._aws_configuration.aws_secretKey self._execute_s3_upload( file_body=files, diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index 84c075ffbd3f4..06247454b6706 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -22,6 +22,7 @@ from marshmallow import fields, Schema, validate, validates, validates_schema from marshmallow.validate import Length, Range, ValidationError from pytz import all_timezones +from superset.reports.notifications.s3 import S3SubTypes from superset.reports.models import ( ReportCreationMethod, @@ -211,6 +212,9 @@ class ReportSchedulePostSchema(Schema): dump_default=None, ) force_screenshot = fields.Boolean(dump_default=False) + aws_key = fields.String(default=None, missing=None) + aws_secret_key = fields.String(default=None, missing=None) + aws_s3_types = fields.String(default=None, missing=None) custom_width = fields.Integer( metadata={ "description": _("Custom width of the screenshot in pixels"), @@ -251,6 +255,23 @@ def validate_report_references( # pylint: disable=unused-argument raise ValidationError( {"database": ["Database reference is not allowed on a report"]} ) + @validates_schema + def validate_aws_fields( + self, data, **kwargs + ): # pylint: disable=unused-argument,no-self-use + + if ( + data["recipients"][0]["type"] == ReportRecipientType.S3 + and data["aws_S3_types"] == S3SubTypes.S3_CRED + ): + if data["aws_key"] is None or data["aws_secretKey"] is None: + raise ValidationError( + { + "aws credentials": [ + "Both AWS keys and Aws secret keys are required" + ] + } + ) class ReportSchedulePutSchema(Schema): @@ -350,6 +371,9 @@ class ReportSchedulePutSchema(Schema): required=False, default=None, ) + aws_key = fields.String(default=None, missing=None) + aws_secret_key = fields.String(default=None, missing=None) + aws_s3_types = fields.String(default=None, missing=None) @validates("custom_width") def validate_custom_width( From 30e204058341f40da5e179e1ab8b004e0769e375 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 17:04:01 +0000 Subject: [PATCH 03/26] corrected the required changes --- superset/commands/report/execute.py | 3 +-- superset/reports/notifications/base.py | 1 - superset/reports/notifications/s3.py | 4 ++-- superset/reports/schemas.py | 4 ++-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py index b8aa6a8bb3653..f56e6f2a932f1 100644 --- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -396,13 +396,12 @@ def _get_notification_content(self) -> NotificationContent: header_data=header_data, ) def _get_aws_configuration(self) -> AwsConfiguration: - # pylint: disable=invalid-name aws_key = self._report_schedule.aws_key aws_secret_key = self._report_schedule.aws_secret_key aws_s3_types = self._report_schedule.aws_s3_types return AwsConfiguration( - aws_key=aws_key, aws_secretKey=aws_secret_key, aws_S3_types=aws_s3_types + aws_key=aws_key, aws_secret_key=aws_secret_key, aws_s3_types=aws_s3_types ) def _send( diff --git a/superset/reports/notifications/base.py b/superset/reports/notifications/base.py index 0121860f4eec8..f07b4d549487d 100644 --- a/superset/reports/notifications/base.py +++ b/superset/reports/notifications/base.py @@ -36,7 +36,6 @@ class NotificationContent: @dataclass class AwsConfiguration: - # pylint: disable=invalid-name aws_key: Optional[str] = None aws_secret_key: Optional[str] = None aws_s3_types: Optional[str] = None diff --git a/superset/reports/notifications/s3.py b/superset/reports/notifications/s3.py index 71cf4c1f3e66b..73c5da8fb74dd 100644 --- a/superset/reports/notifications/s3.py +++ b/superset/reports/notifications/s3.py @@ -82,14 +82,14 @@ def send(self): files = self._get_inline_files() file_type = "csv" if self._content.csv else "png" bucket_name = json.loads(self._recipient.recipient_config_json)["target"] - s3_Subtype = self._aws_configuration.aws_S3_types + s3_Subtype = self._aws_configuration.aws_s3_types try: if s3_Subtype == S3SubTypes.S3_CRED: aws_access_key_id = self._aws_configuration.aws_key - aws_secret_access_key = self._aws_configuration.aws_secretKey + aws_secret_access_key = self._aws_configuration.aws_secret_key self._execute_s3_upload( file_body=files, diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index 06247454b6706..8c597c8dc56f8 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -262,9 +262,9 @@ def validate_aws_fields( if ( data["recipients"][0]["type"] == ReportRecipientType.S3 - and data["aws_S3_types"] == S3SubTypes.S3_CRED + and data["aws_s3_types"] == S3SubTypes.S3_CRED ): - if data["aws_key"] is None or data["aws_secretKey"] is None: + if data["aws_key"] is None or data["aws_secret_key"] is None: raise ValidationError( { "aws credentials": [ From bacf6ac8ddfc53fde61fa4b3ab8606a4901b9d52 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 17:09:16 +0000 Subject: [PATCH 04/26] new changes --- superset/views/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/views/base.py b/superset/views/base.py index 145b996396c58..86c7df390a82f 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -391,7 +391,7 @@ def cached_common_bootstrap_data( # pylint: disable=unused-argument for k in FRONTEND_CONF_KEYS } - isAwsConfigured = ( + is_aws_configured = ( get_feature_flags()["ENABLE_AWS"] if "ENABLE_AWS" in get_feature_flags() else False @@ -402,7 +402,7 @@ def cached_common_bootstrap_data( # pylint: disable=unused-argument frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"].append( ReportRecipientType.SLACK ) - if isAwsConfigured: + if is_aws_configured: frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"].append( ReportRecipientType.S3 ) From 8fa4f61fd7f7b64ecbb61bd4422ff8e71d92a683 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 17:39:22 +0000 Subject: [PATCH 05/26] Added formatter --- docker/pythonpath_dev/superset_config.py | 4 ++-- superset/commands/report/execute.py | 8 ++++---- ...4925b0889720_added_aws_s3_columns_to_reports_model.py | 7 ++++--- superset/reports/api.py | 4 ++-- superset/reports/notifications/__init__.py | 2 +- superset/reports/notifications/base.py | 6 ++++-- superset/reports/notifications/s3.py | 2 +- superset/reports/schemas.py | 9 ++++++--- 8 files changed, 24 insertions(+), 18 deletions(-) diff --git a/docker/pythonpath_dev/superset_config.py b/docker/pythonpath_dev/superset_config.py index 196c7ea42a156..fbef92fcfbfc1 100644 --- a/docker/pythonpath_dev/superset_config.py +++ b/docker/pythonpath_dev/superset_config.py @@ -104,8 +104,8 @@ class CeleryConfig: FEATURE_FLAGS = {"ALERT_REPORTS": True, "ENABLE_AWS": True} ALERT_REPORTS_NOTIFICATION_DRY_RUN = False # AWS Credentials -AWS_ACCESS_KEY = '####' -AWS_SECRET_KEY = '####' +AWS_ACCESS_KEY = "####" +AWS_SECRET_KEY = "####" # # Optionally import superset_config_docker.py (which will have been included on diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py index f56e6f2a932f1..8cfaf7b67dcda 100644 --- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -395,6 +395,7 @@ def _get_notification_content(self) -> NotificationContent: embedded_data=embedded_data, header_data=header_data, ) + def _get_aws_configuration(self) -> AwsConfiguration: aws_key = self._report_schedule.aws_key aws_secret_key = self._report_schedule.aws_secret_key @@ -417,7 +418,6 @@ def _send( notification_errors: list[SupersetError] = [] for recipient in recipients: if recipient.type == ReportRecipientType.S3: - # pylint: disable=invalid-name aws_configuration = self._get_aws_configuration() notification = create_notification( recipient, notification_content, aws_configuration @@ -441,9 +441,9 @@ def _send( SupersetError( message=ex.message, error_type=SupersetErrorType.REPORT_NOTIFICATION_ERROR, - level=ErrorLevel.ERROR - if ex.status >= 500 - else ErrorLevel.WARNING, + level=( + ErrorLevel.ERROR if ex.status >= 500 else ErrorLevel.WARNING + ), ) ) if notification_errors: diff --git a/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py b/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py index 5b433ebf87563..7a7ffb4655a92 100755 --- a/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py +++ b/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py @@ -23,14 +23,15 @@ """ # revision identifiers, used by Alembic. -revision = '4925b0889720' -down_revision = 'be1b217cd8cd' +revision = "4925b0889720" +down_revision = "be1b217cd8cd" from alembic import op import sqlalchemy as sa from sqlalchemy.dialects import postgresql from sqlalchemy_utils import EncryptedType + def upgrade(): op.add_column( @@ -50,4 +51,4 @@ def upgrade(): def downgrade(): op.drop_column("report_schedule", "aws_key") op.drop_column("report_schedule", "aws_secret_key") - op.drop_column("report_schedule", "aws_s3_types") \ No newline at end of file + op.drop_column("report_schedule", "aws_s3_types") diff --git a/superset/reports/api.py b/superset/reports/api.py index 313a1239c0289..629bccd929403 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -121,7 +121,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]: "working_timeout", "aws_secret_key", "aws_key", - "aws_s3_types" + "aws_s3_types", ] show_select_columns = show_columns + [ "chart.datasource_id", @@ -156,7 +156,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]: "type", "aws_secret_key", "aws_key", - "aws_s3_types" + "aws_s3_types", ] add_columns = [ "active", diff --git a/superset/reports/notifications/__init__.py b/superset/reports/notifications/__init__.py index 19f930e98713e..6bb102ac060fc 100644 --- a/superset/reports/notifications/__init__.py +++ b/superset/reports/notifications/__init__.py @@ -28,7 +28,7 @@ def create_notification( recipient: ReportRecipients, notification_content: NotificationContent, - aws_configuration: AwsConfiguration = None + aws_configuration: AwsConfiguration = None, ) -> BaseNotification: """ Notification polymorphic factory diff --git a/superset/reports/notifications/base.py b/superset/reports/notifications/base.py index f07b4d549487d..f24dc244ea748 100644 --- a/superset/reports/notifications/base.py +++ b/superset/reports/notifications/base.py @@ -34,6 +34,7 @@ class NotificationContent: url: Optional[str] = None # url to chart/dashboard for this screenshot embedded_data: Optional[pd.DataFrame] = None + @dataclass class AwsConfiguration: aws_key: Optional[str] = None @@ -61,9 +62,10 @@ def __init_subclass__(cls, *args: Any, **kwargs: Any) -> None: cls.plugins.append(cls) def __init__( - self, recipient: ReportRecipients, + self, + recipient: ReportRecipients, content: NotificationContent, - aws_configuration: AwsConfiguration = None + aws_configuration: AwsConfiguration = None, ) -> None: self._recipient = recipient self._content = content diff --git a/superset/reports/notifications/s3.py b/superset/reports/notifications/s3.py index 73c5da8fb74dd..f83fa424b8fec 100644 --- a/superset/reports/notifications/s3.py +++ b/superset/reports/notifications/s3.py @@ -128,4 +128,4 @@ def send(self): ";".join([error.message for error in ex.errors]) ) from ex except Exception as ex: - raise NotificationError(str(ex)) from ex \ No newline at end of file + raise NotificationError(str(ex)) from ex diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index 8c597c8dc56f8..4083949bcbf77 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -255,10 +255,13 @@ def validate_report_references( # pylint: disable=unused-argument raise ValidationError( {"database": ["Database reference is not allowed on a report"]} ) + @validates_schema - def validate_aws_fields( - self, data, **kwargs - ): # pylint: disable=unused-argument,no-self-use + def validate_aws_fields( # pylint: disable=unused-argument + self, + data: dict[str, Any], + **kwargs: Any, + ) -> None: if ( data["recipients"][0]["type"] == ReportRecipientType.S3 From 18ebd0f6503ffdd8f32dc016ef772a3b4f7b4148 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 17:43:43 +0000 Subject: [PATCH 06/26] added license --- superset/reports/notifications/s3.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/superset/reports/notifications/s3.py b/superset/reports/notifications/s3.py index f83fa424b8fec..76b8444bf7f7a 100644 --- a/superset/reports/notifications/s3.py +++ b/superset/reports/notifications/s3.py @@ -1,3 +1,19 @@ +# 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. import datetime import json import logging From 7716627e781e9adb0cb3bff4d0628e34e700a6d0 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 18:12:43 +0000 Subject: [PATCH 07/26] added pytest for schemas_test --- tests/unit_tests/reports/schemas_test.py | 54 ++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/unit_tests/reports/schemas_test.py b/tests/unit_tests/reports/schemas_test.py index 0fab6d11b9a70..e067aa0ce31ba 100644 --- a/tests/unit_tests/reports/schemas_test.py +++ b/tests/unit_tests/reports/schemas_test.py @@ -43,6 +43,7 @@ def test_report_post_schema_custom_width_validation(mocker: MockFixture) -> None "crontab": "* * * * *", "timezone": "America/Los_Angeles", "custom_width": 100, + "recipients": [{"type": "Email"}], } ) @@ -55,6 +56,7 @@ def test_report_post_schema_custom_width_validation(mocker: MockFixture) -> None "active": True, "crontab": "* * * * *", "timezone": "America/Los_Angeles", + "recipients": [{"type": "Email"}], } ) @@ -68,8 +70,60 @@ def test_report_post_schema_custom_width_validation(mocker: MockFixture) -> None "crontab": "* * * * *", "timezone": "America/Los_Angeles", "custom_width": 1000, + "recipients": [{"type": "Email"}], } ) assert excinfo.value.messages == { "custom_width": ["Screenshot width must be between 100px and 200px"] } + +def test_report_post_schema_custom_width_validation_with_aws_fields(mocker: MockFixture) -> None: + """ + Test the custom width validation along with AWS fields validation. + """ + current_app = mocker.patch("superset.reports.schemas.current_app") + current_app.config = { + "ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH": 100, + "ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH": 200, + } + valid_s3_types = ["AWS_S3_credentials", "AWS_S3_pyconfig", "AWS_S3_IAM"] + schema = ReportSchedulePostSchema() + + for s3_type in valid_s3_types: + # Test with valid AWS credentials and custom_width for each valid S3 type + schema.load( + { + "type": "Report", + "name": "A report", + "description": "My report", + "active": True, + "crontab": "* * * * *", + "timezone": "America/Los_Angeles", + "custom_width": 100, + "recipients": [{"type": "S3"}], + "aws_key": "valid_key", + "aws_secret_key": "valid_secret_key", + "aws_s3_types": s3_type, + } + ) + + # Next, let's test with invalid AWS credentials and custom_width for S3 recipient type + with pytest.raises(ValidationError) as excinfo: + schema.load( + { + "type": "Report", + "name": "A report", + "description": "My report", + "active": True, + "crontab": "* * * * *", + "timezone": "America/Los_Angeles", + "custom_width": 1000, + "recipients": [{"type": "S3"}], + "aws_key": None, # Invalid, missing AWS key + "aws_secret_key": None, # Invalid, missing AWS secret key + "aws_s3_types": "valid_s3_type", + } + ) + assert excinfo.value.messages == { + "aws credentials": ["Both AWS keys and Aws secret keys are required"] + } \ No newline at end of file From 2f8edb017f4c54e2e915644e030f6cd15033423d Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 18:22:46 +0000 Subject: [PATCH 08/26] Custom width test case --- tests/unit_tests/reports/schemas_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit_tests/reports/schemas_test.py b/tests/unit_tests/reports/schemas_test.py index e067aa0ce31ba..555e6c3546445 100644 --- a/tests/unit_tests/reports/schemas_test.py +++ b/tests/unit_tests/reports/schemas_test.py @@ -107,7 +107,7 @@ def test_report_post_schema_custom_width_validation_with_aws_fields(mocker: Mock } ) - # Next, let's test with invalid AWS credentials and custom_width for S3 recipient type + with pytest.raises(ValidationError) as excinfo: schema.load( { @@ -119,11 +119,11 @@ def test_report_post_schema_custom_width_validation_with_aws_fields(mocker: Mock "timezone": "America/Los_Angeles", "custom_width": 1000, "recipients": [{"type": "S3"}], - "aws_key": None, # Invalid, missing AWS key - "aws_secret_key": None, # Invalid, missing AWS secret key - "aws_s3_types": "valid_s3_type", + "aws_key": "valid_key", + "aws_secret_key": "valid_secret_key", # Invalid, missing AWS secret key + "aws_s3_types": s3_type, } ) assert excinfo.value.messages == { - "aws credentials": ["Both AWS keys and Aws secret keys are required"] + "custom_width": ["Screenshot width must be between 100px and 200px"] } \ No newline at end of file From 999bb8bb05d3f32a0d12a5ab6061342e0d878415 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 18:52:57 +0000 Subject: [PATCH 09/26] updated schemas validation --- superset/reports/schemas.py | 12 ++++++++++++ tests/unit_tests/reports/schemas_test.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index 4083949bcbf77..cb9d197378f40 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -263,6 +263,18 @@ def validate_aws_fields( # pylint: disable=unused-argument **kwargs: Any, ) -> None: + if ( + data["recipients"][0]["type"] == ReportRecipientType.S3 + and data["aws_s3_types"] not in vars(S3SubTypes).values() + ): + + raise ValidationError( + { + "aws s3 types": [ + "Not a supported aws s3 sub types" + ] + } + ) if ( data["recipients"][0]["type"] == ReportRecipientType.S3 and data["aws_s3_types"] == S3SubTypes.S3_CRED diff --git a/tests/unit_tests/reports/schemas_test.py b/tests/unit_tests/reports/schemas_test.py index 555e6c3546445..377b2cced31f0 100644 --- a/tests/unit_tests/reports/schemas_test.py +++ b/tests/unit_tests/reports/schemas_test.py @@ -120,7 +120,7 @@ def test_report_post_schema_custom_width_validation_with_aws_fields(mocker: Mock "custom_width": 1000, "recipients": [{"type": "S3"}], "aws_key": "valid_key", - "aws_secret_key": "valid_secret_key", # Invalid, missing AWS secret key + "aws_secret_key": "valid_secret_key", "aws_s3_types": s3_type, } ) From 6332d64ebdcd2d350b16198b7478b26e1f6890a1 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 19:20:35 +0000 Subject: [PATCH 10/26] pylint changes --- superset/commands/base.py | 2 +- superset/reports/schemas.py | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/superset/commands/base.py b/superset/commands/base.py index 8b330d0669c0b..d2efcda4fe838 100644 --- a/superset/commands/base.py +++ b/superset/commands/base.py @@ -58,7 +58,7 @@ def populate_owners(owner_ids: Optional[list[int]] = None) -> list[User]: return populate_owner_list(owner_ids, default_to_user=True) -class UpdateMixin: # pylint: disable=too-few-public-methods +class UpdateMixin: @staticmethod def populate_owners(owner_ids: Optional[list[int]] = None) -> list[User]: """ diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index cb9d197378f40..6927da8e6ae52 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -267,14 +267,10 @@ def validate_aws_fields( # pylint: disable=unused-argument data["recipients"][0]["type"] == ReportRecipientType.S3 and data["aws_s3_types"] not in vars(S3SubTypes).values() ): - - raise ValidationError( - { - "aws s3 types": [ - "Not a supported aws s3 sub types" - ] - } - ) + + raise ValidationError( + {"aws s3 types": ["Not a supported aws s3 sub types"]} + ) if ( data["recipients"][0]["type"] == ReportRecipientType.S3 and data["aws_s3_types"] == S3SubTypes.S3_CRED From 6899d43414ca0cc04d154e98e2a33e10bc057b9a Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 19:36:15 +0000 Subject: [PATCH 11/26] formatted changes --- ..._4925b0889720_added_aws_s3_columns_to_reports_model.py | 2 +- superset/reports/notifications/base.py | 2 +- superset/reports/schemas.py | 2 +- tests/unit_tests/reports/schemas_test.py | 8 +++++--- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py b/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py index 7a7ffb4655a92..47fdc0b4c32ce 100755 --- a/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py +++ b/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py @@ -26,8 +26,8 @@ revision = "4925b0889720" down_revision = "be1b217cd8cd" -from alembic import op import sqlalchemy as sa +from alembic import op from sqlalchemy.dialects import postgresql from sqlalchemy_utils import EncryptedType diff --git a/superset/reports/notifications/base.py b/superset/reports/notifications/base.py index f24dc244ea748..c35523a12dd2b 100644 --- a/superset/reports/notifications/base.py +++ b/superset/reports/notifications/base.py @@ -65,7 +65,7 @@ def __init__( self, recipient: ReportRecipients, content: NotificationContent, - aws_configuration: AwsConfiguration = None, + aws_configuration: Optional[AwsConfiguration] = None, ) -> None: self._recipient = recipient self._content = content diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index 6927da8e6ae52..bf1fd4b69fb23 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -22,7 +22,6 @@ from marshmallow import fields, Schema, validate, validates, validates_schema from marshmallow.validate import Length, Range, ValidationError from pytz import all_timezones -from superset.reports.notifications.s3 import S3SubTypes from superset.reports.models import ( ReportCreationMethod, @@ -31,6 +30,7 @@ ReportScheduleType, ReportScheduleValidatorType, ) +from superset.reports.notifications.s3 import S3SubTypes openapi_spec_methods_override = { "get": {"get": {"summary": "Get a report schedule"}}, diff --git a/tests/unit_tests/reports/schemas_test.py b/tests/unit_tests/reports/schemas_test.py index 377b2cced31f0..83cea76c18028 100644 --- a/tests/unit_tests/reports/schemas_test.py +++ b/tests/unit_tests/reports/schemas_test.py @@ -77,7 +77,10 @@ def test_report_post_schema_custom_width_validation(mocker: MockFixture) -> None "custom_width": ["Screenshot width must be between 100px and 200px"] } -def test_report_post_schema_custom_width_validation_with_aws_fields(mocker: MockFixture) -> None: + +def test_report_post_schema_custom_width_validation_with_aws_fields( + mocker: MockFixture, +) -> None: """ Test the custom width validation along with AWS fields validation. """ @@ -107,7 +110,6 @@ def test_report_post_schema_custom_width_validation_with_aws_fields(mocker: Mock } ) - with pytest.raises(ValidationError) as excinfo: schema.load( { @@ -126,4 +128,4 @@ def test_report_post_schema_custom_width_validation_with_aws_fields(mocker: Mock ) assert excinfo.value.messages == { "custom_width": ["Screenshot width must be between 100px and 200px"] - } \ No newline at end of file + } From c771db30f11aaeebe01d646ce364114276998919 Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 20:19:11 +0000 Subject: [PATCH 12/26] added pre-commit changes --- ...0_added_aws_s3_columns_to_reports_model.py | 1 - superset/reports/notifications/__init__.py | 2 +- superset/reports/notifications/base.py | 2 +- superset/reports/notifications/s3.py | 32 +++++++++---------- superset/reports/schemas.py | 2 -- 5 files changed, 17 insertions(+), 22 deletions(-) diff --git a/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py b/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py index 47fdc0b4c32ce..99d2225d2c418 100755 --- a/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py +++ b/superset/migrations/versions/2024-03-18_15-23_4925b0889720_added_aws_s3_columns_to_reports_model.py @@ -33,7 +33,6 @@ def upgrade(): - op.add_column( "report_schedule", sa.Column("aws_key", EncryptedType(sa.String(1024)), nullable=True), diff --git a/superset/reports/notifications/__init__.py b/superset/reports/notifications/__init__.py index 6bb102ac060fc..a679a0bbc7c75 100644 --- a/superset/reports/notifications/__init__.py +++ b/superset/reports/notifications/__init__.py @@ -28,7 +28,7 @@ def create_notification( recipient: ReportRecipients, notification_content: NotificationContent, - aws_configuration: AwsConfiguration = None, + aws_configuration: AwsConfiguration = None, # type: ignore ) -> BaseNotification: """ Notification polymorphic factory diff --git a/superset/reports/notifications/base.py b/superset/reports/notifications/base.py index c35523a12dd2b..ecf4071fdd033 100644 --- a/superset/reports/notifications/base.py +++ b/superset/reports/notifications/base.py @@ -65,7 +65,7 @@ def __init__( self, recipient: ReportRecipients, content: NotificationContent, - aws_configuration: Optional[AwsConfiguration] = None, + aws_configuration: AwsConfiguration = None, # type: ignore ) -> None: self._recipient = recipient self._content = content diff --git a/superset/reports/notifications/s3.py b/superset/reports/notifications/s3.py index 76b8444bf7f7a..130fb8b2ee664 100644 --- a/superset/reports/notifications/s3.py +++ b/superset/reports/notifications/s3.py @@ -18,6 +18,7 @@ import json import logging from io import BytesIO +from typing import Any, Optional from uuid import uuid4 import boto3 @@ -25,7 +26,7 @@ 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.base import AwsConfiguration, BaseNotification from superset.reports.notifications.exceptions import NotificationError logger = logging.getLogger(__name__) @@ -45,7 +46,7 @@ class S3Notification(BaseNotification): # pylint: disable=too-few-public-method # pylint: disable= too-many-arguments, invalid-name type = ReportRecipientType.S3 - def _get_inline_files(self): + def _get_inline_files(self) -> dict[Any, Any]: current_datetime = datetime.datetime.now() formatted_date = current_datetime.strftime("%Y-%m-%d") report_name = self._content.name @@ -62,16 +63,16 @@ def _get_inline_files(self): for screenshot in self._content.screenshots } return images - return [] + return {} def _execute_s3_upload( self, - file_body, - bucket_name, - contentType, - aws_access_key_id=None, - aws_secret_access_key=None, - ): + file_body: dict[Any, Any], + bucket_name: str, + contentType: str, + aws_access_key_id: Optional[str] = None, + aws_secret_access_key: Optional[str] = None, + ) -> None: for key, file in file_body.items(): file = BytesIO(file) s3 = boto3.client( @@ -94,16 +95,14 @@ def _execute_s3_upload( self._content.header_data, ) - def send(self): + def send(self) -> None: files = self._get_inline_files() file_type = "csv" if self._content.csv else "png" bucket_name = json.loads(self._recipient.recipient_config_json)["target"] - s3_Subtype = self._aws_configuration.aws_s3_types + s3_subtype = self._aws_configuration.aws_s3_types try: - - if s3_Subtype == S3SubTypes.S3_CRED: - + if s3_subtype == S3SubTypes.S3_CRED: aws_access_key_id = self._aws_configuration.aws_key aws_secret_access_key = self._aws_configuration.aws_secret_key @@ -115,13 +114,12 @@ def send(self): aws_secret_access_key=aws_secret_access_key, ) - elif s3_Subtype == S3SubTypes.S3_ROLE: + elif s3_subtype == S3SubTypes.S3_ROLE: self._execute_s3_upload( file_body=files, bucket_name=bucket_name, contentType=file_type ) - elif s3_Subtype == S3SubTypes.S3_CONFIG: - + elif s3_subtype == S3SubTypes.S3_CONFIG: aws_access_key_id = app.config["AWS_ACCESS_KEY"] aws_secret_access_key = app.config["AWS_SECRET_KEY"] diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index bf1fd4b69fb23..a75fbe234823d 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -262,12 +262,10 @@ def validate_aws_fields( # pylint: disable=unused-argument data: dict[str, Any], **kwargs: Any, ) -> None: - if ( data["recipients"][0]["type"] == ReportRecipientType.S3 and data["aws_s3_types"] not in vars(S3SubTypes).values() ): - raise ValidationError( {"aws s3 types": ["Not a supported aws s3 sub types"]} ) From ee2a769b34e14762f9a6dfdf99aef3450571e9fd Mon Sep 17 00:00:00 2001 From: Deepak Date: Mon, 18 Mar 2024 20:27:28 +0000 Subject: [PATCH 13/26] pylint error resolved --- superset/reports/notifications/s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/reports/notifications/s3.py b/superset/reports/notifications/s3.py index 130fb8b2ee664..442a3a00fc9a5 100644 --- a/superset/reports/notifications/s3.py +++ b/superset/reports/notifications/s3.py @@ -26,7 +26,7 @@ from superset import app from superset.exceptions import SupersetErrorsException from superset.reports.models import ReportRecipientType -from superset.reports.notifications.base import AwsConfiguration, BaseNotification +from superset.reports.notifications.base import BaseNotification from superset.reports.notifications.exceptions import NotificationError logger = logging.getLogger(__name__) From 50eac36b96ef60acd0bb57889fceea7123cf7e50 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 05:53:41 +0000 Subject: [PATCH 14/26] snake case for s3 file --- superset/reports/notifications/s3.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/superset/reports/notifications/s3.py b/superset/reports/notifications/s3.py index 442a3a00fc9a5..e87e49daa9469 100644 --- a/superset/reports/notifications/s3.py +++ b/superset/reports/notifications/s3.py @@ -69,7 +69,7 @@ def _execute_s3_upload( self, file_body: dict[Any, Any], bucket_name: str, - contentType: str, + content_type: str, aws_access_key_id: Optional[str] = None, aws_secret_access_key: Optional[str] = None, ) -> None: @@ -86,7 +86,7 @@ def _execute_s3_upload( key, ExtraArgs={ "Metadata": {"Content-Disposition": "inline"}, - "ContentType": contentType, + "content_type": content_type, }, ) @@ -109,14 +109,14 @@ def send(self) -> None: self._execute_s3_upload( file_body=files, bucket_name=bucket_name, - contentType=file_type, + content_type=file_type, aws_access_key_id=aws_access_key_id, aws_secret_access_key=aws_secret_access_key, ) elif s3_subtype == S3SubTypes.S3_ROLE: self._execute_s3_upload( - file_body=files, bucket_name=bucket_name, contentType=file_type + file_body=files, bucket_name=bucket_name, content_type=file_type ) elif s3_subtype == S3SubTypes.S3_CONFIG: @@ -126,7 +126,7 @@ def send(self) -> None: self._execute_s3_upload( file_body=files, bucket_name=bucket_name, - contentType=file_type, + content_type=file_type, aws_access_key_id=aws_access_key_id, aws_secret_access_key=aws_secret_access_key, ) From e7dc57095c49a6641326a691fa2d2dc397eaee31 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 06:16:48 +0000 Subject: [PATCH 15/26] Testing integration --- tests/integration_tests/reports/api_tests.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index 1b74d11f5a0a5..963e5d4ec503f 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -981,6 +981,7 @@ def test_create_multiple_creation_method_report_schedule_charts(self): "crontab": "0 9 * * *", "working_timeout": 3600, "chart": chart.id, + "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -997,6 +998,7 @@ def test_create_multiple_creation_method_report_schedule_charts(self): "crontab": "0 9 * * *", "working_timeout": 3600, "chart": chart.id, + "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1039,6 +1041,7 @@ def test_create_multiple_creation_method_report_schedule_dashboards(self): "crontab": "0 9 * * *", "working_timeout": 3600, "dashboard": dashboard.id, + "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1055,6 +1058,7 @@ def test_create_multiple_creation_method_report_schedule_dashboards(self): "crontab": "0 9 * * *", "working_timeout": 3600, "dashboard": dashboard.id, + "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1098,6 +1102,7 @@ def test_create_report_schedule_chart_dash_validation(self): "chart": chart.id, "dashboard": dashboard.id, "database": example_db.id, + "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1121,6 +1126,7 @@ def test_create_report_schedule_chart_db_validation(self): "creation_method": ReportCreationMethod.ALERTS_REPORTS, "crontab": "0 9 * * *", "chart": chart.id, + "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1148,6 +1154,7 @@ def test_create_report_schedule_relations_exist(self): "crontab": "0 9 * * *", "chart": chart_max_id + 1, "database": database_max_id + 1, + "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1157,6 +1164,7 @@ def test_create_report_schedule_relations_exist(self): "message": { "chart": "Chart does not exist", "database": "Database does not exist", + "recipients": [{"type": "Email"}], } } @@ -1170,6 +1178,7 @@ def test_create_report_schedule_relations_exist(self): "creation_method": ReportCreationMethod.ALERTS_REPORTS, "dashboard": dashboard_max_id + 1, "database": examples_db.id, + "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") From b4dfaf3c04312b149b991e7e63b8cfff2ab5f39f Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 06:44:57 +0000 Subject: [PATCH 16/26] reverted changes --- tests/integration_tests/reports/api_tests.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index 963e5d4ec503f..1b74d11f5a0a5 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -981,7 +981,6 @@ def test_create_multiple_creation_method_report_schedule_charts(self): "crontab": "0 9 * * *", "working_timeout": 3600, "chart": chart.id, - "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -998,7 +997,6 @@ def test_create_multiple_creation_method_report_schedule_charts(self): "crontab": "0 9 * * *", "working_timeout": 3600, "chart": chart.id, - "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1041,7 +1039,6 @@ def test_create_multiple_creation_method_report_schedule_dashboards(self): "crontab": "0 9 * * *", "working_timeout": 3600, "dashboard": dashboard.id, - "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1058,7 +1055,6 @@ def test_create_multiple_creation_method_report_schedule_dashboards(self): "crontab": "0 9 * * *", "working_timeout": 3600, "dashboard": dashboard.id, - "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1102,7 +1098,6 @@ def test_create_report_schedule_chart_dash_validation(self): "chart": chart.id, "dashboard": dashboard.id, "database": example_db.id, - "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1126,7 +1121,6 @@ def test_create_report_schedule_chart_db_validation(self): "creation_method": ReportCreationMethod.ALERTS_REPORTS, "crontab": "0 9 * * *", "chart": chart.id, - "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1154,7 +1148,6 @@ def test_create_report_schedule_relations_exist(self): "crontab": "0 9 * * *", "chart": chart_max_id + 1, "database": database_max_id + 1, - "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1164,7 +1157,6 @@ def test_create_report_schedule_relations_exist(self): "message": { "chart": "Chart does not exist", "database": "Database does not exist", - "recipients": [{"type": "Email"}], } } @@ -1178,7 +1170,6 @@ def test_create_report_schedule_relations_exist(self): "creation_method": ReportCreationMethod.ALERTS_REPORTS, "dashboard": dashboard_max_id + 1, "database": examples_db.id, - "recipients": [{"type": "Email"}], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") From da19ce163bb75b8257f6edff2608d6c0ea952c86 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 10:13:34 +0000 Subject: [PATCH 17/26] test integration --- tests/integration_tests/base_tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration_tests/base_tests.py b/tests/integration_tests/base_tests.py index 7b8d8506c3680..f56cdd1455faa 100644 --- a/tests/integration_tests/base_tests.py +++ b/tests/integration_tests/base_tests.py @@ -85,6 +85,7 @@ def post_assert_metric( with patch.object( BaseSupersetModelRestApi, "incr_stats", return_value=None ) as mock_method: + print("The data is of payload>>>>>>>>>>>>>>>>>>", data) rv = client.post(uri, json=data) if 200 <= rv.status_code < 400: mock_method.assert_called_once_with("success", func_name) From 64194933b6de05ab08c6b8cda7d4ce0d96e923e0 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 13:49:25 +0000 Subject: [PATCH 18/26] Testing api_tests --- tests/integration_tests/reports/api_tests.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index 1b74d11f5a0a5..bcda15da92a95 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -981,6 +981,16 @@ def test_create_multiple_creation_method_report_schedule_charts(self): "crontab": "0 9 * * *", "working_timeout": 3600, "chart": chart.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -997,6 +1007,16 @@ def test_create_multiple_creation_method_report_schedule_charts(self): "crontab": "0 9 * * *", "working_timeout": 3600, "chart": chart.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") From f11a4544915ca968f93b704dbfe089d07dabd90d Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 14:03:50 +0000 Subject: [PATCH 19/26] test 2 api_test --- tests/integration_tests/reports/api_tests.py | 60 ++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index bcda15da92a95..c18923436d1ed 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -1059,6 +1059,16 @@ def test_create_multiple_creation_method_report_schedule_dashboards(self): "crontab": "0 9 * * *", "working_timeout": 3600, "dashboard": dashboard.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1075,6 +1085,16 @@ def test_create_multiple_creation_method_report_schedule_dashboards(self): "crontab": "0 9 * * *", "working_timeout": 3600, "dashboard": dashboard.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1118,6 +1138,16 @@ def test_create_report_schedule_chart_dash_validation(self): "chart": chart.id, "dashboard": dashboard.id, "database": example_db.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1141,6 +1171,16 @@ def test_create_report_schedule_chart_db_validation(self): "creation_method": ReportCreationMethod.ALERTS_REPORTS, "crontab": "0 9 * * *", "chart": chart.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1168,6 +1208,16 @@ def test_create_report_schedule_relations_exist(self): "crontab": "0 9 * * *", "chart": chart_max_id + 1, "database": database_max_id + 1, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -1190,6 +1240,16 @@ def test_create_report_schedule_relations_exist(self): "creation_method": ReportCreationMethod.ALERTS_REPORTS, "dashboard": dashboard_max_id + 1, "database": examples_db.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") From 540575b1f7b10024f6a065ef49291859bd9b3c21 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 14:16:42 +0000 Subject: [PATCH 20/26] Test3 api_test --- tests/integration_tests/reports/api_tests.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index c18923436d1ed..6c8712ba88225 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -688,6 +688,16 @@ def test_create_report_schedule_schema(self): "crontab": "0 9 * * *", "chart": chart.id, "database": example_db.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") From 2b6da354211d78b9fede6e092e26dd6effb7b908 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 14:29:00 +0000 Subject: [PATCH 21/26] test4 api_test --- tests/integration_tests/reports/api_tests.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index 6c8712ba88225..b553893f19137 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -641,6 +641,16 @@ def test_create_report_schedule_uniqueness(self): "crontab": "0 9 * * *", "chart": chart.id, "database": example_db.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") @@ -656,6 +666,16 @@ def test_create_report_schedule_uniqueness(self): "crontab": "0 9 * * *", "creation_method": ReportCreationMethod.ALERTS_REPORTS, "chart": chart.id, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.client.post(uri, json=report_schedule_data) From 8f85f370c3d0a20f03c6e858b873ff7b2dbfd576 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 14:42:58 +0000 Subject: [PATCH 22/26] Test5 api_test --- tests/integration_tests/reports/api_tests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index b553893f19137..f4544c0db7254 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -370,6 +370,9 @@ def test_get_list_report_schedule(self): "recipients", "timezone", "type", + 'aws_key', + 'aws_s3_types', + 'aws_secret_key', ] assert rv.status_code == 200 data = json.loads(rv.data.decode("utf-8")) From 929d75670487fc45c274f1d4722915f53fc2c626 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 15:17:39 +0000 Subject: [PATCH 23/26] test7 api-test --- tests/integration_tests/reports/api_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index f4544c0db7254..9a4a54c7c35fa 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -350,6 +350,9 @@ def test_get_list_report_schedule(self): expected_fields = [ "active", + 'aws_key', + 'aws_s3_types', + 'aws_secret_key', "changed_by", "changed_on", "changed_on_delta_humanized", @@ -370,9 +373,6 @@ def test_get_list_report_schedule(self): "recipients", "timezone", "type", - 'aws_key', - 'aws_s3_types', - 'aws_secret_key', ] assert rv.status_code == 200 data = json.loads(rv.data.decode("utf-8")) From 6019ae608607333b2d95f4698e444efdb9a86e28 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 15:29:22 +0000 Subject: [PATCH 24/26] test8 api_test --- tests/integration_tests/reports/api_tests.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index 9a4a54c7c35fa..dad08dc47564a 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -350,9 +350,9 @@ def test_get_list_report_schedule(self): expected_fields = [ "active", - 'aws_key', - 'aws_s3_types', - 'aws_secret_key', + "aws_key", + "aws_s3_types", + "aws_secret_key", "changed_by", "changed_on", "changed_on_delta_humanized", @@ -985,6 +985,16 @@ def test_no_dashboard_report_schedule_schema(self): "description": "description", "creation_method": ReportCreationMethod.DASHBOARDS, "crontab": "0 9 * * *", + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") From b83cf42e8a862d9892cb25fca9cd1d84d9b4197d Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 15:40:10 +0000 Subject: [PATCH 25/26] test 9 api_test --- tests/integration_tests/reports/api_tests.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index dad08dc47564a..e4a368ccd1ee7 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -958,6 +958,16 @@ def test_unsaved_report_schedule_schema(self): "creation_method": ReportCreationMethod.CHARTS, "crontab": "0 9 * * *", "chart": 0, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "target@superset.org"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "channel"}, + }, + ], } uri = "api/v1/report/" rv = self.post_assert_metric(uri, report_schedule_data, "post") From 6afe4b7c488164663c6f2867ccdc3a235d1a6a61 Mon Sep 17 00:00:00 2001 From: Deepak Date: Tue, 19 Mar 2024 16:15:33 +0000 Subject: [PATCH 26/26] test command report --- .../reports/commands_tests.py | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index 939c9c0cfa9f8..1159448cb3b0c 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -70,6 +70,8 @@ ReportScheduleType, ReportScheduleValidatorType, ReportState, + ReportRecipients, + ReportRecipientType, ) from superset.reports.notifications.exceptions import ( NotificationError, @@ -1967,7 +1969,11 @@ def test_prune_log_soft_time_out(bulk_delete_logs, create_report_email_dashboard @patch("superset.commands.report.execute.create_notification") def test__send_with_client_errors(notification_mock, logger_mock): notification_content = "I am some content" - recipients = ["test@foo.com"] + config_json = {"target": "test@foo.com"} + recipients = [ReportRecipients( + type=ReportRecipientType.EMAIL, + recipient_config_json=json.dumps(config_json), + )] notification_mock.return_value.send.side_effect = NotificationParamException() with pytest.raises(ReportScheduleClientErrorsException) as excinfo: BaseReportState._send(BaseReportState, notification_content, recipients) @@ -1982,7 +1988,11 @@ def test__send_with_client_errors(notification_mock, logger_mock): @patch("superset.commands.report.execute.create_notification") def test__send_with_multiple_errors(notification_mock, logger_mock): notification_content = "I am some content" - recipients = ["test@foo.com", "test2@bar.com"] + config_json = {"target": "test@foo.com,test2@bar.com"} + recipients = [ReportRecipients( + type=ReportRecipientType.EMAIL, + recipient_config_json=json.dumps(config_json), + )] notification_mock.return_value.send.side_effect = [ NotificationParamException(), NotificationError(), @@ -2009,7 +2019,11 @@ def test__send_with_multiple_errors(notification_mock, logger_mock): @patch("superset.commands.report.execute.create_notification") def test__send_with_server_errors(notification_mock, logger_mock): notification_content = "I am some content" - recipients = ["test@foo.com"] + config_json = {"target": "test@foo.com"} + recipients = [ReportRecipients( + type=ReportRecipientType.EMAIL, + recipient_config_json=json.dumps(config_json), + )] notification_mock.return_value.send.side_effect = NotificationError() with pytest.raises(ReportScheduleSystemErrorsException) as excinfo: BaseReportState._send(BaseReportState, notification_content, recipients)