Skip to content

Commit 662f0fa

Browse files
authored
chore(reports): add task for slack channels warm-up (#32585)
1 parent 56bf17f commit 662f0fa

File tree

4 files changed

+84
-22
lines changed

4 files changed

+84
-22
lines changed

superset/config.py

+7
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,7 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
10171017
"superset.tasks.scheduler",
10181018
"superset.tasks.thumbnails",
10191019
"superset.tasks.cache",
1020+
"superset.tasks.slack",
10201021
)
10211022
result_backend = "db+sqlite:///celery_results.sqlite"
10221023
worker_prefetch_multiplier = 1
@@ -1048,6 +1049,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
10481049
# "schedule": crontab(minute="*", hour="*"),
10491050
# "kwargs": {"retention_period_days": 180},
10501051
# },
1052+
# Uncomment to enable Slack channel cache warm-up
1053+
# "slack.cache_channels": {
1054+
# "task": "slack.cache_channels",
1055+
# "schedule": crontab(minute="0", hour="*"),
1056+
# },
10511057
}
10521058

10531059

@@ -1490,6 +1496,7 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument # noq
14901496
# Slack API token for the superset reports, either string or callable
14911497
SLACK_API_TOKEN: Callable[[], str] | str | None = None
14921498
SLACK_PROXY = None
1499+
SLACK_CACHE_TIMEOUT = int(timedelta(days=1).total_seconds())
14931500

14941501
# The webdriver to use for generating reports. Use one of the following
14951502
# firefox

superset/tasks/slack.py

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
import logging
18+
19+
from flask import current_app
20+
21+
from superset.extensions import celery_app
22+
from superset.utils.slack import get_channels
23+
24+
logger = logging.getLogger(__name__)
25+
26+
27+
@celery_app.task(name="slack.cache_channels")
28+
def cache_channels() -> None:
29+
try:
30+
get_channels(
31+
force=True, cache_timeout=current_app.config["SLACK_CACHE_TIMEOUT"]
32+
)
33+
except Exception as ex:
34+
logger.exception("An error occurred while caching Slack channels: %s", ex)
35+
raise

superset/utils/slack.py

+16-9
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818

1919
import logging
20-
from typing import Any, Optional
20+
from typing import Callable, Optional
2121

2222
from flask import current_app
2323
from slack_sdk import WebClient
@@ -60,7 +60,7 @@ def get_slack_client() -> WebClient:
6060
key="slack_conversations_list",
6161
cache=cache_manager.cache,
6262
)
63-
def get_channels(limit: int, extra_params: dict[str, Any]) -> list[SlackChannelSchema]:
63+
def get_channels() -> list[SlackChannelSchema]:
6464
"""
6565
Retrieves a list of all conversations accessible by the bot
6666
from the Slack API, and caches results (to avoid rate limits).
@@ -71,11 +71,12 @@ def get_channels(limit: int, extra_params: dict[str, Any]) -> list[SlackChannelS
7171
client = get_slack_client()
7272
channel_schema = SlackChannelSchema()
7373
channels: list[SlackChannelSchema] = []
74+
extra_params = {"types": ",".join(SlackChannelTypes)}
7475
cursor = None
7576

7677
while True:
7778
response = client.conversations_list(
78-
limit=limit, cursor=cursor, exclude_archived=True, **extra_params
79+
limit=999, cursor=cursor, exclude_archived=True, **extra_params
7980
)
8081
channels.extend(
8182
channel_schema.load(channel) for channel in response.data["channels"]
@@ -89,7 +90,6 @@ def get_channels(limit: int, extra_params: dict[str, Any]) -> list[SlackChannelS
8990

9091
def get_channels_with_search(
9192
search_string: str = "",
92-
limit: int = 999,
9393
types: Optional[list[SlackChannelTypes]] = None,
9494
exact_match: bool = False,
9595
force: bool = False,
@@ -99,18 +99,25 @@ def get_channels_with_search(
9999
all channels and filter them ourselves
100100
This will search by slack name or id
101101
"""
102-
extra_params = {}
103-
extra_params["types"] = ",".join(types) if types else None
104102
try:
105103
channels = get_channels(
106-
limit=limit,
107-
extra_params=extra_params,
108104
force=force,
109-
cache_timeout=86400,
105+
cache_timeout=current_app.config["SLACK_CACHE_TIMEOUT"],
110106
)
111107
except (SlackClientError, SlackApiError) as ex:
112108
raise SupersetException(f"Failed to list channels: {ex}") from ex
113109

110+
if types and not len(types) == len(SlackChannelTypes):
111+
conditions: list[Callable[[SlackChannelSchema], bool]] = []
112+
if SlackChannelTypes.PUBLIC in types:
113+
conditions.append(lambda channel: not channel["is_private"])
114+
if SlackChannelTypes.PRIVATE in types:
115+
conditions.append(lambda channel: channel["is_private"])
116+
117+
channels = [
118+
channel for channel in channels if any(cond(channel) for cond in conditions)
119+
]
120+
114121
# The search string can be multiple channels separated by commas
115122
if search_string:
116123
search_array = recipients_string_to_list(search_string)

tests/unit_tests/utils/slack_test.py

+26-13
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import pytest
1919

20-
from superset.utils.slack import get_channels_with_search
20+
from superset.utils.slack import get_channels_with_search, SlackChannelTypes
2121

2222

2323
class MockResponse:
@@ -150,15 +150,35 @@ def test_handle_slack_client_error_listing_channels(self, mocker):
150150
The server responded with: missing scope: channels:read"""
151151
)
152152

153-
def test_filter_channels_by_specified_types(self, mocker):
153+
@pytest.mark.parametrize(
154+
"types, expected_channel_ids",
155+
[
156+
([SlackChannelTypes.PUBLIC], {"public_channel_id"}),
157+
([SlackChannelTypes.PRIVATE], {"private_channel_id"}),
158+
(
159+
[SlackChannelTypes.PUBLIC, SlackChannelTypes.PRIVATE],
160+
{"public_channel_id", "private_channel_id"},
161+
),
162+
([], {"public_channel_id", "private_channel_id"}),
163+
],
164+
)
165+
def test_filter_channels_by_specified_types(
166+
self, types: list[SlackChannelTypes], expected_channel_ids: set[str], mocker
167+
):
154168
mock_data = {
155169
"channels": [
156170
{
157-
"id": "C12345",
158-
"name": "general",
171+
"id": "public_channel_id",
172+
"name": "open",
159173
"is_member": False,
160174
"is_private": False,
161175
},
176+
{
177+
"id": "private_channel_id",
178+
"name": "secret",
179+
"is_member": False,
180+
"is_private": True,
181+
},
162182
],
163183
"response_metadata": {"next_cursor": None},
164184
}
@@ -168,15 +188,8 @@ def test_filter_channels_by_specified_types(self, mocker):
168188
mock_client.conversations_list.return_value = mock_response_instance
169189
mocker.patch("superset.utils.slack.get_slack_client", return_value=mock_client)
170190

171-
result = get_channels_with_search(types=["public"])
172-
assert result == [
173-
{
174-
"id": "C12345",
175-
"name": "general",
176-
"is_member": False,
177-
"is_private": False,
178-
}
179-
]
191+
result = get_channels_with_search(types=types)
192+
assert {channel["id"] for channel in result} == expected_channel_ids
180193

181194
def test_handle_pagination_multiple_pages(self, mocker):
182195
mock_data_page1 = {

0 commit comments

Comments
 (0)