sadpandajoe opened a new pull request, #39914:
URL: https://github.com/apache/superset/pull/39914
### SUMMARY
Deprecates the legacy Slack v1 integration for Alerts and Reports and adds
bulletproof unit-test coverage for `SlackV2Notification` ahead of v1 removal in
the next major.
**Why now.** Slack retired the `files.upload` endpoint in 2025, so v1 sends
that include screenshots, CSVs, or PDFs already fail at the API level — only
text-only `chat_postMessage` sends still succeed via the legacy path. Existing
recipients with the `channels:read` scope already auto-upgrade to `SlackV2` on
first send via the `update_report_schedule_slack_v2` flow; the only thing
keeping the upgrade from running out of the box was `ALERT_REPORT_SLACK_V2`
defaulting to `False`.
**What this change does:**
- Flips the `ALERT_REPORT_SLACK_V2` default to `True` (the docs JSON
re-syncs from `config.py` automatically via the `feature-flags-sync` pre-commit
hook).
- Adds one-shot `DeprecationWarning` + `logger.warning` emissions when the
v1 path still runs — both the flag-off and the missing-`channels:read` paths.
The scope-missing `logger.warning` continues to fire every send so operators
see the actionable scope hint in report-execution logs.
- Updates the stale `# TODO: Remove in 6.0.0` comment on `SlackNotification`
with an accurate deprecation note (6.0 already shipped).
- Adds an UPDATING.md entry under `## Next` with operator action items
(grant `channels:read`, recipients auto-upgrade on next send, v1 removal
targeted for the next major).
**Bulletproof v2 test coverage** added in
`tests/unit_tests/reports/notifications/slack_tests.py` and
`tests/unit_tests/utils/slack_test.py`:
- `files_upload_v2` invocation with PNG (single + multiple), CSV, and PDF —
asserting `channel`, `file`, `title`, `filename`, and `initial_comment`
- Multi-channel fan-out (3 channels × 2 files = 6 uploads) and text-only
multi-channel `chat_postMessage`
- Inline-file precedence (CSV beats screenshots beats PDF)
- Parametrized exception mapping across all 7 `slack_sdk` error types → the
4 `NotificationException` subclasses
- Statsd `.ok` and `.warning` gauge emission via the `@statsd_gauge`
decorator
- `execution_id` propagation from `g.logs_context` to the success log, plus
the falsy-fallback path
- End-to-end auto-upgrade round-trip: v1 `SLACK` recipient with channel
names → `SlackV1NotificationError` → `update_report_schedule_slack_v2` rewrites
the row to channel IDs → fresh `SlackV2Notification` fast-paths the next send
with no resolver call
- `should_use_v2_api()` warning behavior: `DeprecationWarning` emitted
exactly once across multiple calls in both flag-off and scope-missing paths
**Real bug surfaced (not fixed here):** `SlackV2Notification.send()` is
decorated with `@backoff.on_exception(SlackApiError, ..., max_tries=5)`, but
the function catches every `SlackApiError` internally and re-raises as
`NotificationUnprocessableException` before backoff can see it — so no retries
actually fire. The test
`test_v2_send_backoff_decorator_does_not_retry_swallowed_slack_api_errors`
locks in current behavior (`call_count == 1`) with a docstring explaining the
design issue. Worth a separate change to fix the retry config.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no UI changes.
### TESTING INSTRUCTIONS
```bash
# Unit tests
PYTHONPATH=/path/to/superset_core python3 -m pytest \
tests/unit_tests/reports/ \
tests/unit_tests/commands/report/ \
tests/unit_tests/utils/slack_test.py
```
All 319 tests pass on this branch (29 in `slack_tests.py`, 14 in
`utils/slack_test.py`, plus the existing report unit suite).
To validate the deprecation behavior manually:
1. With `ALERT_REPORT_SLACK_V2: False` set explicitly in
`superset_config.py` and a Slack recipient configured, fire a report. You
should see a single `DeprecationWarning` and `logger.warning` line on the first
send, then no more for that process.
2. With the default (`True`) and a Slack bot lacking `channels:read`, fire a
report. You should see `logger.warning` describing the missing scope on every
send, plus a one-shot `DeprecationWarning`.
3. With the default and a Slack bot that has `channels:read`, the first send
for a `SLACK`-type recipient auto-upgrades the row to `SLACKV2` with channel
IDs (existing behavior — now exercised by the round-trip unit test).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags: `ALERT_REPORT_SLACK_V2` default flipped to
`True`
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [x] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
