You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/07/15 20:25:19 UTC
(superset) branch codex/fix-slack-v1-private-channel-reports created (now 990e067a480)
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a change to branch codex/fix-slack-v1-private-channel-reports
in repository https://gitbox.apache.org/repos/asf/superset.git
at 990e067a480 fix(reports): preserve Slack v1 private-channel text delivery
This branch includes the following new commits:
new 990e067a480 fix(reports): preserve Slack v1 private-channel text delivery
The 1 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) 01/01: fix(reports): preserve Slack v1 private-channel text delivery
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch codex/fix-slack-v1-private-channel-reports
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 990e067a480741217f4eda92fbed504d38d56116
Author: Joe Li <jo...@preset.io>
AuthorDate: Wed Jul 15 13:24:57 2026 -0700
fix(reports): preserve Slack v1 private-channel text delivery
---
UPDATING.md | 2 +
superset/commands/report/execute.py | 36 ++++++-
superset/reports/notifications/slack.py | 22 ++++-
tests/unit_tests/commands/report/execute_test.py | 119 +++++++++++++++++++++++
4 files changed, 171 insertions(+), 8 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 138fe882375..c3ef60c0f04 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -463,6 +463,8 @@ With the flag enabled: `DELETE /api/v1/chart/<id>` no longer hard-deletes the ch
**Importer behavior:** importing a chart YAML whose UUID matches an existing **soft-deleted** chart is treated as an implicit restore-with-update — **and this happens even when `overwrite` is not set**. This is a deliberate asymmetry with active rows: an active chart imported without `overwrite=true` is returned unchanged, but a soft-deleted UUID match is restored *and* has the upload's contents applied regardless of the `overwrite` argument, on the reasoning that re-importing a deleted [...]
+### Slack v1 deprecation and automatic Slack v2 upgrades
+
- [39914](https://github.com/apache/superset/pull/39914) `ALERT_REPORT_SLACK_V2` now defaults to `True` and the legacy Slack v1 integration (`Slack` recipient type, `files.upload` API) is deprecated for removal in the next major. Slack blocked new apps from `files.upload` in May 2024 and fully retired the method for all apps on November 12, 2025; because the v1 path sends files through `files.upload`, v1 file-bearing sends now fail at the API level — only text-only `chat_postMessage` sti [...]
### Soft delete and restore for dashboards
diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py
index 9feac4ed9f2..351c9583f23 100644
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -73,12 +73,13 @@ from superset.reports.models import (
ReportState,
)
from superset.reports.notifications import create_notification
-from superset.reports.notifications.base import NotificationContent
+from superset.reports.notifications.base import BaseNotification, NotificationContent
from superset.reports.notifications.exceptions import (
NotificationError,
NotificationParamException,
SlackV1NotificationError,
)
+from superset.reports.notifications.slack import SlackNotification
from superset.subjects.types import SubjectType
from superset.tasks.utils import get_executor
from superset.utils import json
@@ -936,6 +937,30 @@ class BaseReportState:
header_data=header_data,
)
+ def _upgrade_and_send_slack_notification(
+ self,
+ notification: BaseNotification,
+ recipient: ReportRecipients,
+ notification_content: NotificationContent,
+ ) -> None:
+ """Upgrade a Slack recipient, falling back to text-only v1 delivery."""
+ try:
+ self.update_report_schedule_slack_v2()
+ except UpdateFailedError as update_error:
+ logger.warning(
+ "Slack v2 upgrade unavailable; attempting a text-only Slack v1 "
+ "fallback for this execution: %s",
+ update_error,
+ )
+ if not isinstance(notification, SlackNotification):
+ raise
+ notification.send(force_v1=True)
+ return
+
+ recipient.type = ReportRecipientType.SLACKV2
+ notification = create_notification(recipient, notification_content)
+ notification.send()
+
def _send(
self,
notification_content: NotificationContent,
@@ -966,10 +991,11 @@ class BaseReportState:
logger.info(
"Attempting to upgrade the report to Slackv2: %s", str(ex)
)
- self.update_report_schedule_slack_v2()
- recipient.type = ReportRecipientType.SLACKV2
- notification = create_notification(recipient, notification_content)
- notification.send()
+ self._upgrade_and_send_slack_notification(
+ notification,
+ recipient,
+ notification_content,
+ )
except (
UpdateFailedError,
NotificationParamException,
diff --git a/superset/reports/notifications/slack.py b/superset/reports/notifications/slack.py
index bd527bdd3ce..79ebcb03e91 100644
--- a/superset/reports/notifications/slack.py
+++ b/superset/reports/notifications/slack.py
@@ -21,6 +21,7 @@ from typing import Union
import backoff
from flask import g
+from slack_sdk import WebClient
from slack_sdk.errors import (
BotUserAccessError,
SlackApiError,
@@ -52,6 +53,12 @@ from superset.utils.slack import (
logger = logging.getLogger(__name__)
+_SLACK_V1_FILE_UPLOAD_MESSAGE = (
+ "Slack v1 file uploads are no longer supported because Slack retired "
+ "`files.upload`. Grant the Slack bot both the `channels:read` and "
+ "`groups:read` scopes so the recipient can be upgraded to Slack v2."
+)
+
# Deprecated: Slack v1 will be removed in the next major release. The Slack
# `files.upload` endpoint was retired in 2025, so file-bearing sends already
@@ -88,19 +95,28 @@ class SlackNotification(SlackMixin, BaseNotification): # pylint: disable=too-fe
return ("pdf", [self._content.pdf])
return (None, [])
+ @staticmethod
+ def _send_text(client: WebClient, channel: str, body: str) -> None:
+ """Send a text notification once to each configured channel."""
+ for target in recipients_string_to_list(channel):
+ client.chat_postMessage(channel=target, text=body)
+
@backoff.on_exception(backoff.expo, SlackApiError, factor=10, base=2, max_tries=5)
@statsd_gauge("reports.slack.send")
- def send(self) -> None:
+ def send(self, *, force_v1: bool = False) -> None:
file_type, files = self._get_inline_files()
title = self._content.name
body = self._get_body(content=self._content)
global_logs_context = getattr(g, "logs_context", {}) or {}
# see if the v2 api will work
- if should_use_v2_api():
+ if not force_v1 and should_use_v2_api():
# if we can fetch channels, then raise an error and use the v2 api
raise SlackV1NotificationError
+ if force_v1 and files:
+ raise NotificationParamException(_SLACK_V1_FILE_UPLOAD_MESSAGE)
+
try:
client = get_slack_client()
channel = self._get_channel()
@@ -115,7 +131,7 @@ class SlackNotification(SlackMixin, BaseNotification): # pylint: disable=too-fe
filetype=file_type,
)
else:
- client.chat_postMessage(channel=channel, text=body)
+ self._send_text(client, channel, body)
logger.info(
"Report sent to slack",
extra={
diff --git a/tests/unit_tests/commands/report/execute_test.py b/tests/unit_tests/commands/report/execute_test.py
index c19aa79f587..b736d8058e3 100644
--- a/tests/unit_tests/commands/report/execute_test.py
+++ b/tests/unit_tests/commands/report/execute_test.py
@@ -28,6 +28,7 @@ from superset.app import SupersetApp
from superset.commands.exceptions import UpdateFailedError
from superset.commands.report.exceptions import (
ReportScheduleAlertGracePeriodError,
+ ReportScheduleClientErrorsException,
ReportScheduleCsvFailedError,
ReportScheduleExecuteUnexpectedError,
ReportScheduleExecutorNotFoundError,
@@ -57,6 +58,7 @@ from superset.reports.models import (
ReportSourceFormat,
ReportState,
)
+from superset.reports.notifications.base import NotificationContent
from superset.subjects.types import SubjectType
from superset.utils.core import HeaderDataType
from superset.utils.screenshots import ChartScreenshot
@@ -1738,6 +1740,123 @@ def test_update_recipient_to_slack_v2_no_slack_recipients_is_noop(
mock_search.assert_not_called()
+def test_send_falls_back_to_slack_v1_when_private_channels_upgrade_fails(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """A partial-visibility probe must not block a text-only v1 send."""
+ app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"] = False
+ original_config = json.dumps({"target": "private-a, private-b"})
+ recipient = ReportRecipients(
+ type=ReportRecipientType.SLACK,
+ recipient_config_json=original_config,
+ )
+ report_schedule = ReportSchedule(
+ name="Private channel report",
+ recipients=[recipient],
+ )
+ report_state = BaseReportState(
+ report_schedule,
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ notification_content = NotificationContent(
+ name="Private channel report",
+ header_data={
+ "notification_format": "TEXT",
+ "notification_type": "Report",
+ "editors": [],
+ "notification_source": None,
+ "chart_id": None,
+ "dashboard_id": None,
+ "slack_channels": ["private-a", "private-b"],
+ "execution_id": "execution_id_example",
+ },
+ description="Text-only report",
+ url="https://superset.example/report",
+ )
+ v2_probe = mocker.patch(
+ "superset.reports.notifications.slack.should_use_v2_api",
+ return_value=True,
+ )
+ channel_search = mocker.patch(
+ "superset.commands.report.execute.get_channels_with_search",
+ return_value=[],
+ )
+ slack_client = mocker.patch("superset.reports.notifications.slack.get_slack_client")
+ mocker.patch("superset.reports.notifications.slack.g", logs_context={})
+
+ report_state._send(notification_content, report_schedule.recipients)
+
+ v2_probe.assert_called_once_with()
+ channel_search.assert_called_once()
+ assert slack_client.return_value.chat_postMessage.call_count == 2
+ assert [
+ call.kwargs["channel"]
+ for call in slack_client.return_value.chat_postMessage.call_args_list
+ ] == ["private-a", "private-b"]
+ assert recipient.type == ReportRecipientType.SLACK
+ assert recipient.recipient_config_json == original_config
+
+
+def test_send_does_not_fall_back_to_slack_v1_for_file_uploads(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """A failed v2 migration must not retry a retired v1 file upload."""
+ app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"] = False
+ recipient = ReportRecipients(
+ type=ReportRecipientType.SLACK,
+ recipient_config_json=json.dumps({"target": "private-channel"}),
+ )
+ report_schedule = ReportSchedule(
+ name="Private channel report",
+ recipients=[recipient],
+ )
+ report_state = BaseReportState(
+ report_schedule,
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ notification_content = NotificationContent(
+ name="Private channel report",
+ header_data={
+ "notification_format": "PNG",
+ "notification_type": "Report",
+ "editors": [],
+ "notification_source": None,
+ "chart_id": None,
+ "dashboard_id": None,
+ "slack_channels": ["private-channel"],
+ "execution_id": "execution_id_example",
+ },
+ screenshots=[b"screenshot"],
+ description="File-bearing report",
+ url="https://superset.example/report",
+ )
+ mocker.patch(
+ "superset.reports.notifications.slack.should_use_v2_api",
+ return_value=True,
+ )
+ mocker.patch(
+ "superset.commands.report.execute.get_channels_with_search",
+ return_value=[],
+ )
+ slack_client = mocker.patch("superset.reports.notifications.slack.get_slack_client")
+ mocker.patch("superset.reports.notifications.slack.g", logs_context={})
+
+ with pytest.raises(ReportScheduleClientErrorsException) as exc_info:
+ report_state._send(notification_content, report_schedule.recipients)
+
+ error_message = str(exc_info.value.errors[0].message)
+ assert "Slack v1 file uploads are no longer supported" in error_message
+ assert "`channels:read` and `groups:read`" in error_message
+ slack_client.return_value.files_upload.assert_not_called()
+ slack_client.return_value.chat_postMessage.assert_not_called()
+ assert recipient.type == ReportRecipientType.SLACK
+ assert recipient.recipient_config_json == '{"target": "private-channel"}'
+
+
# ---------------------------------------------------------------------------
# Tier 1: _update_query_context + create_log
# ---------------------------------------------------------------------------