codeant-ai-for-open-source[bot] commented on code in PR #42074:
URL: https://github.com/apache/superset/pull/42074#discussion_r3584594589
##########
superset/commands/report/execute.py:
##########
@@ -187,14 +187,15 @@ def update_report_schedule_slack_v2(self) -> None:
channel_names = (slack_recipients["target"] or
"").replace("#", "")
# we need to ensure that existing reports can also fetch
# ids from private channels
- channels = get_channels_with_search(
+ channels_data = get_channels_with_search(
Review Comment:
**Suggestion:** Add an explicit type annotation for this new local variable
to comply with the type-hint requirement. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This is a newly added local variable in modified Python code, and it is not
type-annotated even though its value can be annotated. That matches the
type-hint rule violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bde1565dd2fa4675b7b6fd3ce5cdf411&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=bde1565dd2fa4675b7b6fd3ce5cdf411&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/report/execute.py
**Line:** 190:190
**Comment:**
*Custom Rule: Add an explicit type annotation for this new local
variable to comply with the type-hint requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=a0c5fa2d70a02eeb607b31206f036a8f2865d76b9c2558ac29bd0d52b3fb01d9&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=a0c5fa2d70a02eeb607b31206f036a8f2865d76b9c2558ac29bd0d52b3fb01d9&reaction=dislike'>π</a>
##########
superset/reports/api.py:
##########
@@ -682,14 +687,29 @@ def slack_channels(self, **kwargs: Any) -> Response:
search_string = params.get("search_string")
types = params.get("types", [])
exact_match = params.get("exact_match", False)
+ cursor = params.get("cursor")
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced
local variable to satisfy the type-hint requirement. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The new local variable `cursor` is introduced in modified Python code
without any type annotation, and it is a value that can be annotated (likely an
optional string). This matches the rule requiring type hints on relevant
variables in new or modified Python code.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cb2cddec698b4d04b5af1976324ae382&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=cb2cddec698b4d04b5af1976324ae382&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/reports/api.py
**Line:** 690:690
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced
local variable to satisfy the type-hint requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=f7b1465d3f2600e390d084d19b9d72cbba704f0d82e46436fad27139ba30a913&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=f7b1465d3f2600e390d084d19b9d72cbba704f0d82e46436fad27139ba30a913&reaction=dislike'>π</a>
##########
superset/tasks/slack.py:
##########
@@ -15,29 +15,160 @@
# specific language governing permissions and limitations
# under the License.
import logging
+from typing import Any, Optional
from flask import current_app
-from superset.extensions import celery_app
-from superset.utils.slack import get_channels
+from superset.extensions import cache_manager, celery_app
+from superset.utils.slack import (
+ _fetch_from_api,
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+ SlackChannelTypes,
+)
logger = logging.getLogger(__name__)
+# Fallback time limits used when the task is dispatched without explicit
+# options (e.g. a bare Celery beat schedule entry). On-demand callers should
+# pass config-driven limits via ``cache_channels.apply_async`` using
+# ``get_cache_warmup_options`` so that ``SLACK_CACHE_WARMUP_TIMEOUT`` is
honored.
+DEFAULT_CACHE_WARMUP_TIME_LIMIT = 300 # 5 minutes
+DEFAULT_CACHE_WARMUP_SOFT_TIME_LIMIT = int(DEFAULT_CACHE_WARMUP_TIME_LIMIT *
0.8)
-@celery_app.task(name="slack.cache_channels")
+
+def get_cache_warmup_options() -> dict[str, int]:
+ """
+ Build Celery ``apply_async`` options with config-driven time limits.
+
+ Read at dispatch time (inside an application context) rather than at import
+ so operator overrides of ``SLACK_CACHE_WARMUP_TIMEOUT`` are respected. The
+ soft limit is derived as 80% of the hard limit so the task can wind down
+ before Celery force-kills it.
+ """
+ time_limit = current_app.config.get(
+ "SLACK_CACHE_WARMUP_TIMEOUT", DEFAULT_CACHE_WARMUP_TIME_LIMIT
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation for `time_limit` so the
config-derived value is guaranteed to match the declared integer return
contract. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This new local variable is introduced without a type annotation even though
it is a config-derived integer value that can be annotated, so it matches the
Python type-hint rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8ee5e835493f424e9ccc562193a3a48a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8ee5e835493f424e9ccc562193a3a48a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tasks/slack.py
**Line:** 49:51
**Comment:**
*Custom Rule: Add an explicit type annotation for `time_limit` so the
config-derived value is guaranteed to match the declared integer return
contract.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=2fbf08f7a07c251aaec39e445ac3879e8fbad142ebaa4de0ed9ceb916a36bdea&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=2fbf08f7a07c251aaec39e445ac3879e8fbad142ebaa4de0ed9ceb916a36bdea&reaction=dislike'>π</a>
##########
superset/commands/report/execute.py:
##########
@@ -187,14 +187,15 @@ def update_report_schedule_slack_v2(self) -> None:
channel_names = (slack_recipients["target"] or
"").replace("#", "")
# we need to ensure that existing reports can also fetch
# ids from private channels
- channels = get_channels_with_search(
+ channels_data = get_channels_with_search(
search_string=channel_names,
types=[
SlackChannelTypes.PRIVATE,
SlackChannelTypes.PUBLIC,
],
exact_match=True,
)
+ channels = channels_data["result"]
Review Comment:
**Suggestion:** Add a concrete type annotation for this derived local
variable so the new code remains fully typed. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This is also a newly introduced local variable in modified Python code, and
it lacks an explicit type hint despite being annotatable. The rule violation is
real in the current file state.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=59cf55ca65d04e06a72bdc5e26b3ce5c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=59cf55ca65d04e06a72bdc5e26b3ce5c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/report/execute.py
**Line:** 198:198
**Comment:**
*Custom Rule: Add a concrete type annotation for this derived local
variable so the new code remains fully typed.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=65642fcbcbe8edbd5975fd818bbd0aa7149ea038064c724c0bd40ca1919b6dbf&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=65642fcbcbe8edbd5975fd818bbd0aa7149ea038064c724c0bd40ca1919b6dbf&reaction=dislike'>π</a>
##########
superset/tasks/slack.py:
##########
@@ -15,29 +15,160 @@
# specific language governing permissions and limitations
# under the License.
import logging
+from typing import Any, Optional
from flask import current_app
-from superset.extensions import celery_app
-from superset.utils.slack import get_channels
+from superset.extensions import cache_manager, celery_app
+from superset.utils.slack import (
+ _fetch_from_api,
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+ SlackChannelTypes,
+)
logger = logging.getLogger(__name__)
+# Fallback time limits used when the task is dispatched without explicit
+# options (e.g. a bare Celery beat schedule entry). On-demand callers should
+# pass config-driven limits via ``cache_channels.apply_async`` using
+# ``get_cache_warmup_options`` so that ``SLACK_CACHE_WARMUP_TIMEOUT`` is
honored.
+DEFAULT_CACHE_WARMUP_TIME_LIMIT = 300 # 5 minutes
+DEFAULT_CACHE_WARMUP_SOFT_TIME_LIMIT = int(DEFAULT_CACHE_WARMUP_TIME_LIMIT *
0.8)
-@celery_app.task(name="slack.cache_channels")
+
+def get_cache_warmup_options() -> dict[str, int]:
+ """
+ Build Celery ``apply_async`` options with config-driven time limits.
+
+ Read at dispatch time (inside an application context) rather than at import
+ so operator overrides of ``SLACK_CACHE_WARMUP_TIMEOUT`` are respected. The
+ soft limit is derived as 80% of the hard limit so the task can wind down
+ before Celery force-kills it.
+ """
+ time_limit = current_app.config.get(
+ "SLACK_CACHE_WARMUP_TIMEOUT", DEFAULT_CACHE_WARMUP_TIME_LIMIT
+ )
+ return {
+ "time_limit": time_limit,
+ "soft_time_limit": int(time_limit * 0.8),
+ }
+
+
+@celery_app.task(
+ name="slack.cache_channels",
+ time_limit=DEFAULT_CACHE_WARMUP_TIME_LIMIT,
+ soft_time_limit=DEFAULT_CACHE_WARMUP_SOFT_TIME_LIMIT,
+)
def cache_channels() -> None:
+ """
+ Celery task to warm up the Slack channels cache.
+
+ This task fetches all Slack channels using pagination and stores them in
cache.
+
+ Respects the following config variables:
+ - SLACK_ENABLE_CACHING: If False, this task does nothing
+ - SLACK_CACHE_TIMEOUT: How long to keep cached data
+ - SLACK_CACHE_MAX_CHANNELS: Maximum number of channels to cache. When the
+ workspace has more channels than this, the cache is truncated and a
+ continuation cursor is stored so paginated lookups can stream the
+ remainder from the Slack API.
+ """
+ enable_caching = current_app.config.get("SLACK_ENABLE_CACHING", True)
+ if not enable_caching:
+ logger.info(
+ "Slack caching disabled (SLACK_ENABLE_CACHING=False), skipping
cache warmup"
+ )
+ return
+
cache_timeout = current_app.config["SLACK_CACHE_TIMEOUT"]
+ max_channels = current_app.config.get("SLACK_CACHE_MAX_CHANNELS", 20000)
retry_count = current_app.config.get("SLACK_API_RATE_LIMIT_RETRY_COUNT", 2)
logger.info(
"Starting Slack channels cache warm-up task "
- "(cache_timeout=%ds, retry_count=%d)",
+ "(cache_timeout=%ds, max_channels=%d, retry_count=%d)",
cache_timeout,
+ max_channels,
retry_count,
)
try:
- get_channels(force=True, cache_timeout=cache_timeout)
+ all_channels: list[Any] = []
+ cursor: Optional[str] = None
+ page_count = 0
+ # When the workspace exceeds ``max_channels`` we cache the first
+ # ``max_channels`` and remember the Slack cursor for the rest so that
+ # in-memory cache pagination can fall back to the API at the boundary.
+ continuation_cursor: Optional[str] = None
+
+ # Fetch directly from the Slack API (bypassing the cache-aware
+ # ``get_channels_with_search``) so a warm cache never feeds the warmup
+ # back into itself and real continuation cursors are produced.
+ while True:
+ page_count += 1
+
+ result = _fetch_from_api(
+ search_string="",
+ types=list(SlackChannelTypes),
+ exact_match=False,
+ cursor=cursor,
+ limit=999,
+ )
+
+ page_channels = result["result"]
+ all_channels.extend(page_channels)
+
+ logger.debug(
+ "Fetched page %d: %d channels (total: %d)",
+ page_count,
+ len(page_channels),
+ len(all_channels),
+ )
+
+ cursor = result.get("next_cursor")
+
+ if not cursor or not result.get("has_more"):
+ break
+
+ if len(all_channels) >= max_channels:
+ continuation_cursor = cursor
+ logger.info(
+ "Reached SLACK_CACHE_MAX_CHANNELS (%d); caching the first "
+ "%d channels and storing a continuation cursor for the
rest.",
+ max_channels,
+ max_channels,
+ )
+ break
+
+ all_channels = all_channels[:max_channels]
+
+ cache_size_mb = len(str(all_channels)) / (1024 * 1024)
Review Comment:
**Suggestion:** Add a type annotation for `cache_size_mb` to keep derived
numeric values explicitly typed. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This derived numeric local is newly introduced and unannotated, even though
it can be explicitly typed as a float, so it violates the type-hint rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0e7004879b1e48ca97e1d9a4f87ffcd7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0e7004879b1e48ca97e1d9a4f87ffcd7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tasks/slack.py
**Line:** 146:146
**Comment:**
*Custom Rule: Add a type annotation for `cache_size_mb` to keep derived
numeric values explicitly typed.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=e3c6fe81129eae0092ab2a024ebea29f33441e4cd0fd25ba6dec9f2bf290dcc9&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=e3c6fe81129eae0092ab2a024ebea29f33441e4cd0fd25ba6dec9f2bf290dcc9&reaction=dislike'>π</a>
##########
superset/tasks/slack.py:
##########
@@ -15,29 +15,160 @@
# specific language governing permissions and limitations
# under the License.
import logging
+from typing import Any, Optional
from flask import current_app
-from superset.extensions import celery_app
-from superset.utils.slack import get_channels
+from superset.extensions import cache_manager, celery_app
+from superset.utils.slack import (
+ _fetch_from_api,
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+ SlackChannelTypes,
+)
logger = logging.getLogger(__name__)
+# Fallback time limits used when the task is dispatched without explicit
+# options (e.g. a bare Celery beat schedule entry). On-demand callers should
+# pass config-driven limits via ``cache_channels.apply_async`` using
+# ``get_cache_warmup_options`` so that ``SLACK_CACHE_WARMUP_TIMEOUT`` is
honored.
+DEFAULT_CACHE_WARMUP_TIME_LIMIT = 300 # 5 minutes
+DEFAULT_CACHE_WARMUP_SOFT_TIME_LIMIT = int(DEFAULT_CACHE_WARMUP_TIME_LIMIT *
0.8)
-@celery_app.task(name="slack.cache_channels")
+
+def get_cache_warmup_options() -> dict[str, int]:
+ """
+ Build Celery ``apply_async`` options with config-driven time limits.
+
+ Read at dispatch time (inside an application context) rather than at import
+ so operator overrides of ``SLACK_CACHE_WARMUP_TIMEOUT`` are respected. The
+ soft limit is derived as 80% of the hard limit so the task can wind down
+ before Celery force-kills it.
+ """
+ time_limit = current_app.config.get(
+ "SLACK_CACHE_WARMUP_TIMEOUT", DEFAULT_CACHE_WARMUP_TIME_LIMIT
+ )
+ return {
+ "time_limit": time_limit,
+ "soft_time_limit": int(time_limit * 0.8),
+ }
+
+
+@celery_app.task(
+ name="slack.cache_channels",
+ time_limit=DEFAULT_CACHE_WARMUP_TIME_LIMIT,
+ soft_time_limit=DEFAULT_CACHE_WARMUP_SOFT_TIME_LIMIT,
+)
def cache_channels() -> None:
+ """
+ Celery task to warm up the Slack channels cache.
+
+ This task fetches all Slack channels using pagination and stores them in
cache.
+
+ Respects the following config variables:
+ - SLACK_ENABLE_CACHING: If False, this task does nothing
+ - SLACK_CACHE_TIMEOUT: How long to keep cached data
+ - SLACK_CACHE_MAX_CHANNELS: Maximum number of channels to cache. When the
+ workspace has more channels than this, the cache is truncated and a
+ continuation cursor is stored so paginated lookups can stream the
+ remainder from the Slack API.
+ """
+ enable_caching = current_app.config.get("SLACK_ENABLE_CACHING", True)
+ if not enable_caching:
+ logger.info(
+ "Slack caching disabled (SLACK_ENABLE_CACHING=False), skipping
cache warmup"
+ )
+ return
+
cache_timeout = current_app.config["SLACK_CACHE_TIMEOUT"]
+ max_channels = current_app.config.get("SLACK_CACHE_MAX_CHANNELS", 20000)
retry_count = current_app.config.get("SLACK_API_RATE_LIMIT_RETRY_COUNT", 2)
logger.info(
"Starting Slack channels cache warm-up task "
- "(cache_timeout=%ds, retry_count=%d)",
+ "(cache_timeout=%ds, max_channels=%d, retry_count=%d)",
cache_timeout,
+ max_channels,
retry_count,
)
try:
- get_channels(force=True, cache_timeout=cache_timeout)
+ all_channels: list[Any] = []
+ cursor: Optional[str] = None
+ page_count = 0
+ # When the workspace exceeds ``max_channels`` we cache the first
+ # ``max_channels`` and remember the Slack cursor for the rest so that
+ # in-memory cache pagination can fall back to the API at the boundary.
+ continuation_cursor: Optional[str] = None
+
+ # Fetch directly from the Slack API (bypassing the cache-aware
+ # ``get_channels_with_search``) so a warm cache never feeds the warmup
+ # back into itself and real continuation cursors are produced.
+ while True:
+ page_count += 1
+
+ result = _fetch_from_api(
+ search_string="",
+ types=list(SlackChannelTypes),
+ exact_match=False,
+ cursor=cursor,
+ limit=999,
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation for `result` to document the
expected API response shape and preserve type safety in downstream indexing.
[custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This newly added local variable is used for dictionary-style access but has
no type annotation; it is a relevant variable that can be annotated under the
type-hint rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a32605299fae411aac66ae99c1d0370a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a32605299fae411aac66ae99c1d0370a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tasks/slack.py
**Line:** 111:117
**Comment:**
*Custom Rule: Add an explicit type annotation for `result` to document
the expected API response shape and preserve type safety in downstream indexing.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=3b792f7cd801b86631e19edfff76346d8e4a526d5d27629bb36e72651dc964da&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=3b792f7cd801b86631e19edfff76346d8e4a526d5d27629bb36e72651dc964da&reaction=dislike'>π</a>
##########
superset/tasks/slack.py:
##########
@@ -15,29 +15,160 @@
# specific language governing permissions and limitations
# under the License.
import logging
+from typing import Any, Optional
from flask import current_app
-from superset.extensions import celery_app
-from superset.utils.slack import get_channels
+from superset.extensions import cache_manager, celery_app
+from superset.utils.slack import (
+ _fetch_from_api,
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+ SlackChannelTypes,
+)
logger = logging.getLogger(__name__)
+# Fallback time limits used when the task is dispatched without explicit
+# options (e.g. a bare Celery beat schedule entry). On-demand callers should
+# pass config-driven limits via ``cache_channels.apply_async`` using
+# ``get_cache_warmup_options`` so that ``SLACK_CACHE_WARMUP_TIMEOUT`` is
honored.
+DEFAULT_CACHE_WARMUP_TIME_LIMIT = 300 # 5 minutes
+DEFAULT_CACHE_WARMUP_SOFT_TIME_LIMIT = int(DEFAULT_CACHE_WARMUP_TIME_LIMIT *
0.8)
-@celery_app.task(name="slack.cache_channels")
+
+def get_cache_warmup_options() -> dict[str, int]:
+ """
+ Build Celery ``apply_async`` options with config-driven time limits.
+
+ Read at dispatch time (inside an application context) rather than at import
+ so operator overrides of ``SLACK_CACHE_WARMUP_TIMEOUT`` are respected. The
+ soft limit is derived as 80% of the hard limit so the task can wind down
+ before Celery force-kills it.
+ """
+ time_limit = current_app.config.get(
+ "SLACK_CACHE_WARMUP_TIMEOUT", DEFAULT_CACHE_WARMUP_TIME_LIMIT
+ )
+ return {
+ "time_limit": time_limit,
+ "soft_time_limit": int(time_limit * 0.8),
+ }
+
+
+@celery_app.task(
+ name="slack.cache_channels",
+ time_limit=DEFAULT_CACHE_WARMUP_TIME_LIMIT,
+ soft_time_limit=DEFAULT_CACHE_WARMUP_SOFT_TIME_LIMIT,
+)
def cache_channels() -> None:
+ """
+ Celery task to warm up the Slack channels cache.
+
+ This task fetches all Slack channels using pagination and stores them in
cache.
+
+ Respects the following config variables:
+ - SLACK_ENABLE_CACHING: If False, this task does nothing
+ - SLACK_CACHE_TIMEOUT: How long to keep cached data
+ - SLACK_CACHE_MAX_CHANNELS: Maximum number of channels to cache. When the
+ workspace has more channels than this, the cache is truncated and a
+ continuation cursor is stored so paginated lookups can stream the
+ remainder from the Slack API.
+ """
+ enable_caching = current_app.config.get("SLACK_ENABLE_CACHING", True)
+ if not enable_caching:
+ logger.info(
+ "Slack caching disabled (SLACK_ENABLE_CACHING=False), skipping
cache warmup"
+ )
+ return
+
cache_timeout = current_app.config["SLACK_CACHE_TIMEOUT"]
+ max_channels = current_app.config.get("SLACK_CACHE_MAX_CHANNELS", 20000)
retry_count = current_app.config.get("SLACK_API_RATE_LIMIT_RETRY_COUNT", 2)
logger.info(
"Starting Slack channels cache warm-up task "
- "(cache_timeout=%ds, retry_count=%d)",
+ "(cache_timeout=%ds, max_channels=%d, retry_count=%d)",
cache_timeout,
+ max_channels,
retry_count,
)
try:
- get_channels(force=True, cache_timeout=cache_timeout)
+ all_channels: list[Any] = []
+ cursor: Optional[str] = None
+ page_count = 0
Review Comment:
**Suggestion:** Add a type annotation to `page_count` to keep loop state
variables explicitly typed. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This loop counter is a newly added Python variable and is not
type-annotated, despite being straightforwardly annotatable as an integer.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c1140594ade9469ba1de68a21d0a6c57&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c1140594ade9469ba1de68a21d0a6c57&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tasks/slack.py
**Line:** 99:99
**Comment:**
*Custom Rule: Add a type annotation to `page_count` to keep loop state
variables explicitly typed.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=60b32adeaa39315aeee4a6d7145e77f02e1fd13d6f6e326accd9d76538977292&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=60b32adeaa39315aeee4a6d7145e77f02e1fd13d6f6e326accd9d76538977292&reaction=dislike'>π</a>
##########
superset/utils/slack.py:
##########
@@ -112,104 +115,305 @@ def get_team_id() -> Optional[str]:
return team_id or None
-def get_channels(
- team_id: Optional[str] = None, **kwargs: Any
-) -> list[SlackChannelSchema]:
- """
- Retrieves a list of all conversations accessible by the bot
- from the Slack API, and caches results (to avoid rate limits).
-
- The Slack API does not provide search so to apply a search use
- get_channels_with_search instead.
-
- :param team_id: workspace (team) ID to target, required for org-scoped
tokens
- on an Enterprise Grid org. Defaults to the configured ``SLACK_TEAM_ID``
- (via get_team_id) when not given. When set it also keys the cache so
that
- distinct workspaces never share cached channel lists; when unset, the
- legacy cache key is used so that upgrading does not invalidate existing
- caches.
- :param kwargs: forwarded to the memoized fetch (``force``,
``cache_timeout``,
- ``cache``).
+def _fetch_channels_without_search(
+ client: WebClient,
+ channel_schema: SlackChannelSchema,
+ types_param: str,
+ cursor: Optional[str],
+ limit: int,
+ cache_full_list: bool = False,
+) -> dict[str, Any]:
+ """Fetch channels without search filtering, paginating for large limits.
+
+ When ``cache_full_list`` is True and the entire workspace is returned in a
+ single page (``has_more`` is False), the complete channel list is written
+ through to the cache so instances without a Celery warmup worker keep the
+ in-memory fast path on subsequent requests. Larger, multi-page workspaces
+ are left to the async warmup task to avoid blocking the request.
"""
- if team_id is None:
- team_id = get_team_id()
- cache_key = "slack_conversations_list"
- if team_id:
- cache_key = f"{cache_key}_{team_id}"
- return _get_channels(cache_key, team_id=team_id, **kwargs)
+ channels: list[SlackChannelSchema] = []
+ slack_cursor = cursor
+ while True:
+ response = client.conversations_list(
+ limit=999,
+ cursor=slack_cursor,
+ exclude_archived=True,
+ types=types_param,
+ )
-@cache_util.memoized_func(
- key="{cache_key}",
- cache=cache_manager.cache,
-)
-def _get_channels(
- cache_key: str, team_id: Optional[str] = None
-) -> list[SlackChannelSchema]:
- client = get_slack_client()
- channel_schema = SlackChannelSchema()
- channels: list[SlackChannelSchema] = []
- extra_params = {"types": _SLACK_CONVERSATION_TYPES}
- if team_id:
- extra_params["team_id"] = team_id
- cursor = None
- page_count = 0
+ page_channels = [
+ channel_schema.load(channel) for channel in
response.data["channels"]
+ ]
+ channels.extend(page_channels)
- logger.info("Starting Slack channels fetch")
+ slack_cursor = response.data.get("response_metadata",
{}).get("next_cursor")
- try:
- while True:
- page_count += 1
+ if not slack_cursor or len(channels) >= limit:
+ break
- response = client.conversations_list(
- limit=999, cursor=cursor, exclude_archived=True, **extra_params
- )
- page_channels = response.data["channels"]
- channels.extend(channel_schema.load(channel) for channel in
page_channels)
-
- logger.debug(
- "Fetched page %d: %d channels (total: %d)",
- page_count,
- len(page_channels),
- len(channels),
- )
+ has_more = bool(slack_cursor)
+
+ # Write-through cache: only when the full, unfiltered workspace fits in the
+ # pages already fetched (has_more is False). ``channels`` is the complete
+ # list here, so it can back the in-memory search/pagination path.
+ if cache_full_list and not has_more:
+ cache_timeout = app.config["SLACK_CACHE_TIMEOUT"]
+ cache_manager.cache.set(
+ SLACK_CHANNELS_CACHE_KEY, channels, timeout=cache_timeout
+ )
+ cache_manager.cache.delete(SLACK_CHANNELS_CONTINUATION_CURSOR_KEY)
+ logger.info(
+ "Warmed Slack channel cache with %d channels without a Celery "
+ "worker (single-page workspace)",
+ len(channels),
+ )
- cursor = response.data.get("response_metadata",
{}).get("next_cursor")
- if not cursor:
+ return {
+ "result": channels[:limit],
+ "next_cursor": slack_cursor,
+ "has_more": has_more,
+ }
+
+
+def _fetch_channels_with_search(
+ client: WebClient,
+ channel_schema: SlackChannelSchema,
+ types_param: str,
+ search_string: str,
+ exact_match: bool,
+ cursor: Optional[str],
+ limit: int,
+) -> dict[str, Any]:
+ """Fetch channels with search filtering, streaming through pages."""
+ matches: list[SlackChannelSchema] = []
+ slack_cursor = cursor
+ search_terms = [
+ term.strip().lower() for term in search_string.split(",") if
term.strip()
+ ]
+
+ while len(matches) < limit:
+ response = client.conversations_list(
+ limit=999,
+ cursor=slack_cursor,
+ exclude_archived=True,
+ types=types_param,
+ )
+
+ for channel_data in response.data["channels"]:
+ channel_name_lower = channel_data["name"].lower()
+ channel_id_lower = channel_data["id"].lower()
+
+ is_match = False
+ for search_term in search_terms:
+ if exact_match:
+ if (
+ search_term == channel_name_lower
+ or search_term == channel_id_lower
+ ):
+ is_match = True
+ break
+ else:
+ if (
+ search_term in channel_name_lower
+ or search_term in channel_id_lower
+ ):
+ is_match = True
+ break
+
+ if is_match:
+ channel = channel_schema.load(channel_data)
+ matches.append(channel)
+
+ if len(matches) >= limit:
break
+ slack_cursor = response.data.get("response_metadata",
{}).get("next_cursor")
+ if not slack_cursor:
+ break
+
+ has_more = bool(slack_cursor) and len(matches) >= limit
+
+ return {
+ "result": matches[:limit],
+ "next_cursor": slack_cursor if has_more else None,
+ "has_more": has_more,
+ }
+
+
+def _fetch_from_cache( # noqa: C901
+ cached_channels: list[SlackChannelSchema],
+ search_string: str,
+ types: Optional[list[SlackChannelTypes]],
+ exact_match: bool,
+ cursor: Optional[str],
+ limit: int,
+) -> Optional[dict[str, Any]]:
+ """
+ Fetch channels from cache with in-memory filtering and pagination.
+
+ This is the fast path - operates entirely in-memory on cached data.
+ Used when cache is available and warm.
+
+ Args:
+ cached_channels: Complete list of all cached channels
+ search_string: Search term(s) for filtering
+ types: Channel types to filter
+ exact_match: If True, search term must exactly match
+ cursor: Cache pagination cursor (format: "cache:N")
+ limit: Maximum channels to return
+
+ Returns:
+ Dict with filtered and paginated results, or None if pagination
+ exceeds cached data (signals need to fall back to API)
+ """
+ # Start with all cached channels
+ channels = list(cached_channels)
+
+ # Filter by channel types if specified
+ if types and len(types) < len(SlackChannelTypes):
+ type_set = set(types)
+ channels = [
+ ch
+ for ch in channels
+ if (
+ (SlackChannelTypes.PRIVATE in type_set and
ch.get("is_private"))
+ or (SlackChannelTypes.PUBLIC in type_set and not
ch.get("is_private"))
+ )
+ ]
+
+ # Filter by search string with comma-separated OR logic
+ if search_string:
+ search_terms = [
+ term.strip().lower() for term in search_string.split(",") if
term.strip()
+ ]
+ filtered = []
Review Comment:
**Suggestion:** Add an explicit type annotation for this list variable to
satisfy the type-hint requirement for newly introduced relevant variables.
[custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The new code introduces a list variable without a type annotation even
though its element type can be annotated from context. This matches the Python
type-hint rule for relevant variables.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6b321953736f4faaace13b4c3757e8cc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6b321953736f4faaace13b4c3757e8cc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/slack.py
**Line:** 290:290
**Comment:**
*Custom Rule: Add an explicit type annotation for this list variable to
satisfy the type-hint requirement for newly introduced relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=4deb827aecfed4ce999a3f64cff36aa0eb1120e3b60bdb8e08a80247ce878141&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=4deb827aecfed4ce999a3f64cff36aa0eb1120e3b60bdb8e08a80247ce878141&reaction=dislike'>π</a>
##########
tests/integration_tests/reports/api_tests.py:
##########
@@ -2975,3 +2975,276 @@ def test_execute_report_schedule_celery_error(self,
mock_execute: Any) -> None:
data = json.loads(rv.data.decode("utf-8"))
assert "Celery" in data["message"]
assert "broker" in data["message"].lower()
+
+ @patch("superset.reports.api.get_channels_with_search")
+ def test_slack_channels_api_without_pagination(self, mock_get_channels):
Review Comment:
**Suggestion:** Add explicit type hints for the patched argument and the
return type in this test method signature. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This new Python test method omits type hints for both the patched argument
and the return type, which violates the rule requiring type hints on new or
modified Python code where they can be annotated.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=abe8cb35dc4c41f0863844e548b4ac4b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=abe8cb35dc4c41f0863844e548b4ac4b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/reports/api_tests.py
**Line:** 2980:2980
**Comment:**
*Custom Rule: Add explicit type hints for the patched argument and the
return type in this test method signature.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=24e58619f719fb55293bc8ee591a038d09e7401fb075e4e4c5c45bf7314bf4de&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=24e58619f719fb55293bc8ee591a038d09e7401fb075e4e4c5c45bf7314bf4de&reaction=dislike'>π</a>
##########
tests/unit_tests/tasks/test_slack.py:
##########
@@ -0,0 +1,134 @@
+# 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.
+
+from typing import Any, Optional
+
+from superset.tasks.slack import (
+ cache_channels,
+ DEFAULT_CACHE_WARMUP_TIME_LIMIT,
+ get_cache_warmup_options,
+)
+from superset.utils.slack import (
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+)
+
+
+def _page(ids: list[str], next_cursor: Optional[str], has_more: bool) ->
dict[str, Any]:
+ return {
+ "result": [{"id": channel_id} for channel_id in ids],
+ "next_cursor": next_cursor,
+ "has_more": has_more,
+ }
+
+
+def _patch_task(mocker, config, fetch_pages):
+ """Patch the task's collaborators and return the mocked cache."""
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = config
+ mock_fetch = mocker.patch(
+ "superset.tasks.slack._fetch_from_api", side_effect=fetch_pages
+ )
+ mock_cache = mocker.patch("superset.tasks.slack.cache_manager")
+ return mock_fetch, mock_cache
+
+
+def test_get_cache_warmup_options_uses_configured_timeout(mocker):
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = {"SLACK_CACHE_WARMUP_TIMEOUT": 600}
+
+ assert get_cache_warmup_options() == {
+ "time_limit": 600,
+ "soft_time_limit": 480,
+ }
+
+
+def test_get_cache_warmup_options_falls_back_to_default(mocker):
Review Comment:
**Suggestion:** Add a type annotation for the fixture argument and an
explicit return type for this test function. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This function is newly added and has no type annotations on the fixture
argument or return type, so it violates the typing rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7d7c7329185849b98a72d2718a168e40&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7d7c7329185849b98a72d2718a168e40&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/tasks/test_slack.py
**Line:** 60:60
**Comment:**
*Custom Rule: Add a type annotation for the fixture argument and an
explicit return type for this test function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=592b7021fabbb5ef572eeb424c61ca22599f0994d14f8779f133388466a36673&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=592b7021fabbb5ef572eeb424c61ca22599f0994d14f8779f133388466a36673&reaction=dislike'>π</a>
##########
tests/unit_tests/tasks/test_slack.py:
##########
@@ -0,0 +1,134 @@
+# 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.
+
+from typing import Any, Optional
+
+from superset.tasks.slack import (
+ cache_channels,
+ DEFAULT_CACHE_WARMUP_TIME_LIMIT,
+ get_cache_warmup_options,
+)
+from superset.utils.slack import (
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+)
+
+
+def _page(ids: list[str], next_cursor: Optional[str], has_more: bool) ->
dict[str, Any]:
+ return {
+ "result": [{"id": channel_id} for channel_id in ids],
+ "next_cursor": next_cursor,
+ "has_more": has_more,
+ }
+
+
+def _patch_task(mocker, config, fetch_pages):
Review Comment:
**Suggestion:** Add explicit type annotations for all parameters and the
return value of this helper function. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This helper function has no type annotations on any parameter or on its
return type, which violates the requirement to add type hints to new Python
code.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=890f7af5e85844a6a9aa3cfc76dba6d8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=890f7af5e85844a6a9aa3cfc76dba6d8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/tasks/test_slack.py
**Line:** 39:39
**Comment:**
*Custom Rule: Add explicit type annotations for all parameters and the
return value of this helper function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=757d75fd7c12add62c793154f022ab7a92e4525f60ae71aed7f4f0e8c1666cc6&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=757d75fd7c12add62c793154f022ab7a92e4525f60ae71aed7f4f0e8c1666cc6&reaction=dislike'>π</a>
##########
tests/unit_tests/tasks/test_slack.py:
##########
@@ -0,0 +1,134 @@
+# 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.
+
+from typing import Any, Optional
+
+from superset.tasks.slack import (
+ cache_channels,
+ DEFAULT_CACHE_WARMUP_TIME_LIMIT,
+ get_cache_warmup_options,
+)
+from superset.utils.slack import (
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+)
+
+
+def _page(ids: list[str], next_cursor: Optional[str], has_more: bool) ->
dict[str, Any]:
+ return {
+ "result": [{"id": channel_id} for channel_id in ids],
+ "next_cursor": next_cursor,
+ "has_more": has_more,
+ }
+
+
+def _patch_task(mocker, config, fetch_pages):
+ """Patch the task's collaborators and return the mocked cache."""
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = config
+ mock_fetch = mocker.patch(
+ "superset.tasks.slack._fetch_from_api", side_effect=fetch_pages
+ )
+ mock_cache = mocker.patch("superset.tasks.slack.cache_manager")
+ return mock_fetch, mock_cache
+
+
+def test_get_cache_warmup_options_uses_configured_timeout(mocker):
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = {"SLACK_CACHE_WARMUP_TIMEOUT": 600}
+
+ assert get_cache_warmup_options() == {
+ "time_limit": 600,
+ "soft_time_limit": 480,
+ }
+
+
+def test_get_cache_warmup_options_falls_back_to_default(mocker):
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = {}
+
+ options = get_cache_warmup_options()
+
+ assert options["time_limit"] == DEFAULT_CACHE_WARMUP_TIME_LIMIT
+ assert options["soft_time_limit"] == int(DEFAULT_CACHE_WARMUP_TIME_LIMIT *
0.8)
+
+
+def test_cache_channels_skips_when_caching_disabled(mocker):
Review Comment:
**Suggestion:** Provide type hints for the function parameter and return
type on this newly added test. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The test signature is untyped, with no annotation for the fixture parameter
or the return value, which is a real violation of the stated rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4f7d32e81de64c509bdc1fc04004b76b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4f7d32e81de64c509bdc1fc04004b76b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/tasks/test_slack.py
**Line:** 70:70
**Comment:**
*Custom Rule: Provide type hints for the function parameter and return
type on this newly added test.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=d8a81bde19acf7dc1b41c61d654e02e7fd9d451842f4e07cd384e32786da062d&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=d8a81bde19acf7dc1b41c61d654e02e7fd9d451842f4e07cd384e32786da062d&reaction=dislike'>π</a>
##########
tests/unit_tests/tasks/test_slack.py:
##########
@@ -0,0 +1,134 @@
+# 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.
+
+from typing import Any, Optional
+
+from superset.tasks.slack import (
+ cache_channels,
+ DEFAULT_CACHE_WARMUP_TIME_LIMIT,
+ get_cache_warmup_options,
+)
+from superset.utils.slack import (
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+)
+
+
+def _page(ids: list[str], next_cursor: Optional[str], has_more: bool) ->
dict[str, Any]:
+ return {
+ "result": [{"id": channel_id} for channel_id in ids],
+ "next_cursor": next_cursor,
+ "has_more": has_more,
+ }
+
+
+def _patch_task(mocker, config, fetch_pages):
+ """Patch the task's collaborators and return the mocked cache."""
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = config
+ mock_fetch = mocker.patch(
+ "superset.tasks.slack._fetch_from_api", side_effect=fetch_pages
+ )
+ mock_cache = mocker.patch("superset.tasks.slack.cache_manager")
+ return mock_fetch, mock_cache
+
+
+def test_get_cache_warmup_options_uses_configured_timeout(mocker):
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = {"SLACK_CACHE_WARMUP_TIMEOUT": 600}
+
+ assert get_cache_warmup_options() == {
+ "time_limit": 600,
+ "soft_time_limit": 480,
+ }
+
+
+def test_get_cache_warmup_options_falls_back_to_default(mocker):
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = {}
+
+ options = get_cache_warmup_options()
+
+ assert options["time_limit"] == DEFAULT_CACHE_WARMUP_TIME_LIMIT
+ assert options["soft_time_limit"] == int(DEFAULT_CACHE_WARMUP_TIME_LIMIT *
0.8)
+
+
+def test_cache_channels_skips_when_caching_disabled(mocker):
+ _, mock_cache = _patch_task(mocker, {"SLACK_ENABLE_CACHING": False},
fetch_pages=[])
+
+ cache_channels()
+
+ mock_cache.cache.set.assert_not_called()
+
+
+def test_cache_channels_caches_all_and_clears_continuation(mocker):
Review Comment:
**Suggestion:** Add type annotations for this test functionβs arguments and
return type. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This newly added test function lacks type hints for its parameter and return
type, so it violates the custom typing rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=94bc67fcc636478fa8eaa1bf1bc3e27a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=94bc67fcc636478fa8eaa1bf1bc3e27a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/tasks/test_slack.py
**Line:** 78:78
**Comment:**
*Custom Rule: Add type annotations for this test functionβs arguments
and return type.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=9bd8d6501fc293497eda3c3babd30361d7baca823c1ff10799ca55ecdfbfba38&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=9bd8d6501fc293497eda3c3babd30361d7baca823c1ff10799ca55ecdfbfba38&reaction=dislike'>π</a>
##########
tests/unit_tests/utils/slack_test.py:
##########
@@ -520,3 +875,615 @@ def test_propagates_non_sdk_errors_from_probe(self,
mocker):
with pytest.raises(RuntimeError):
should_use_v2_api()
+
+
+class TestGetChannelsWithSearchPagination:
+ def test_partial_page_results(self, mocker):
Review Comment:
**Suggestion:** Annotate this new method with explicit parameter and return
type hints to comply with the typing rule. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This is a newly added test method and it lacks type hints for the `mocker`
argument and return type, which is a real violation of the typing rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=40c4a3204c1245a7a832072d0c7d94ef&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=40c4a3204c1245a7a832072d0c7d94ef&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/slack_test.py
**Line:** 881:881
**Comment:**
*Custom Rule: Annotate this new method with explicit parameter and
return type hints to comply with the typing rule.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=30e70b3fff731626fae191809ec34b7da355894bbe4a57f1a374e03b3394d4ed&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=30e70b3fff731626fae191809ec34b7da355894bbe4a57f1a374e03b3394d4ed&reaction=dislike'>π</a>
##########
tests/unit_tests/utils/slack_test.py:
##########
@@ -75,15 +102,136 @@ def test_handle_empty_search_string(self, mocker):
mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
result = get_channels_with_search(search_string="")
- assert result == [{"name": "general", "id": "C12345"}]
+ assert result == {
+ "result": [
+ {
+ "name": "general",
+ "id": "C12345",
+ "is_private": False,
+ "is_member": True,
+ }
+ ],
+ "next_cursor": None,
+ "has_more": False,
+ }
+
Review Comment:
**Suggestion:** Add type hints to this new test method signature, including
the `mocker` fixture type and a `None` return type. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This is a newly added Python test method and it omits type hints for the
`mocker` parameter and return type, which violates the Python type-hint rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8abfb477cef64045a8e51fc011645c28&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8abfb477cef64045a8e51fc011645c28&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/slack_test.py
**Line:** 117:117
**Comment:**
*Custom Rule: Add type hints to this new test method signature,
including the `mocker` fixture type and a `None` return type.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=9fb126744931b2475974a6132063a1eb148b494a39269bb2b81857a98682e2f9&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=9fb126744931b2475974a6132063a1eb148b494a39269bb2b81857a98682e2f9&reaction=dislike'>π</a>
##########
tests/unit_tests/utils/slack_test.py:
##########
@@ -520,3 +875,615 @@ def test_propagates_non_sdk_errors_from_probe(self,
mocker):
with pytest.raises(RuntimeError):
should_use_v2_api()
+
+
+class TestGetChannelsWithSearchPagination:
+ def test_partial_page_results(self, mocker):
+ """Test pagination with partial page (less than limit)"""
+
+ # Only 50 channels returned (less than default 100 limit)
+ channels = [
+ {
+ "name": f"channel-{i}",
+ "id": f"C{i}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(50)
+ ]
+
+ mock_data = {
+ "channels": channels,
+ "response_metadata": {"next_cursor": None},
+ }
+ mock_response = MockResponse(mock_data)
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ result = get_channels_with_search(limit=100)
+
+ # Should return all 50 channels
+ assert len(result["result"]) == 50
+ # Should indicate no more results
+ assert result["has_more"] is False
+ assert result["next_cursor"] is None
+
+ def test_streaming_search_stops_when_limit_reached(self, mocker):
+ """Test that streaming search stops immediately when limit is
reached"""
+
+ # First page with 60 matching channels
+ page1_channels = [
+ {"name": f"test-{i}", "id": f"C{i}", "is_private": False,
"is_member": True}
+ for i in range(60)
+ ]
+ # Second page with 60 more matching channels (should not be fully
processed)
+ page2_channels = [
+ {
+ "name": f"test-{i}",
+ "id": f"C{i + 60}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(60)
+ ]
+
+ mock_data_page1 = {
+ "channels": page1_channels,
+ "response_metadata": {"next_cursor": "page2"},
+ }
+ mock_data_page2 = {
+ "channels": page2_channels,
+ "response_metadata": {"next_cursor": "page3"},
+ }
+
+ mock_response_page1 = MockResponse(mock_data_page1)
+ mock_response_page2 = MockResponse(mock_data_page2)
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.side_effect = [
+ mock_response_page1,
+ mock_response_page2,
+ ]
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Request limit of 100
+ result = get_channels_with_search(search_string="test", limit=100)
+
+ # Should return exactly 100 channels (60 from page1 + 40 from page2)
+ assert len(result["result"]) == 100
+ # Should indicate more results available (next cursor points to page3)
+ assert result["has_more"] is True
+ assert result["next_cursor"] == "page3"
+
+ def test_cursor_format_with_special_characters(self, mocker):
+ """Test that cursor with special characters is handled correctly"""
+
+ # Slack cursors are base64 encoded strings that might contain special
chars
+ special_cursor = "dGVhbTpDMDYxRkE1UEw="
+
+ mock_data = {
+ "channels": [
+ {"name": "test", "id": "C123", "is_private": False,
"is_member": True},
+ ],
+ "response_metadata": {"next_cursor": None},
+ }
+ mock_response = MockResponse(mock_data)
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Call with special cursor
+ get_channels_with_search(cursor=special_cursor, limit=100)
+
+ # Verify cursor was passed to Slack API
+ mock_client.conversations_list.assert_called_once()
+ call_kwargs = mock_client.conversations_list.call_args[1]
+ assert call_kwargs["cursor"] == special_cursor
+
+ def test_empty_channel_list_response(self, mocker):
+ """Test handling of empty channels list from Slack API"""
+
+ mock_data = {
+ "channels": [],
+ "response_metadata": {"next_cursor": None},
+ }
+ mock_response = MockResponse(mock_data)
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ result = get_channels_with_search()
+
+ assert result == {
+ "result": [],
+ "next_cursor": None,
+ "has_more": False,
+ }
+
+ def test_custom_limit_parameter(self, mocker):
+ """Test that custom limit parameter is respected"""
+
+ all_channels = [
+ {
+ "name": f"channel-{i}",
+ "id": f"C{i}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(200)
+ ]
+
+ # Mock should respect the limit parameter (simulating Slack API
behavior)
+ def mock_conversations_list(**kwargs):
+ limit = kwargs.get("limit", 100)
+ return MockResponse(
+ {
+ "channels": all_channels[:limit],
+ "response_metadata": {"next_cursor": "next_page"},
+ }
+ )
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.side_effect = mock_conversations_list
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Request custom limit of 50
+ result = get_channels_with_search(limit=50)
+
+ # Should return exactly 50 channels
+ assert len(result["result"]) == 50
+ assert result["has_more"] is True
+
+ def test_non_search_pagination_over_1000_limit(self, mocker):
+ """Test non-search queries paginate correctly for limits > 1000"""
+ # Create 2500 channels
+ all_channels = [
+ {
+ "name": f"channel-{i}",
+ "id": f"C{i}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(2500)
+ ]
+
+ call_count = 0
+
+ def mock_conversations_list(**kwargs):
+ nonlocal call_count
+ limit = kwargs.get("limit", 999)
+ cursor = kwargs.get("cursor")
+
+ if cursor is None:
+ start = 0
+ elif cursor == "cursor_999":
+ start = 999
+ elif cursor == "cursor_1998":
+ start = 1998
+ else:
+ start = 2500
+
+ end = min(start + limit, 2500)
+ next_cursor = f"cursor_{end}" if end < 2500 else None
+
+ call_count += 1
+ return MockResponse(
+ {
+ "channels": all_channels[start:end],
+ "response_metadata": {"next_cursor": next_cursor},
+ }
+ )
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.side_effect = mock_conversations_list
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Request 1500 channels (requires 2 pages of 999 each)
+ result = get_channels_with_search(limit=1500)
+
+ # Should return exactly 1500 channels
+ assert len(result["result"]) == 1500
+ assert result["has_more"] is True
+ assert result["next_cursor"] == "cursor_1998"
+ # Should have made 2 API calls (999 + 501)
+ assert call_count == 2
+
+ def test_search_with_exact_match_optimization(self, mocker):
+ """Test exact match search uses optimized string comparison"""
+ channels = [
+ {"name": "test", "id": "C1", "is_private": False, "is_member":
True},
+ {"name": "test-dev", "id": "C2", "is_private": False, "is_member":
True},
+ {"name": "testing", "id": "C3", "is_private": False, "is_member":
True},
+ ]
+ mock_data = {"channels": channels, "response_metadata":
{"next_cursor": None}}
+
+ mock_response = MockResponse(mock_data)
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ result = get_channels_with_search(
+ search_string="TEST", exact_match=True, limit=100
+ )
+
+ # Only "test" should match (case-insensitive exact match)
+ assert len(result["result"]) == 1
+ assert result["result"][0]["name"] == "test"
+
+ def test_search_substring_match_optimization(self, mocker):
+ """Test substring search uses optimized string comparison"""
+ channels = [
+ {"name": "prod-api", "id": "C1", "is_private": False, "is_member":
True},
+ {"name": "dev-api", "id": "C2", "is_private": False, "is_member":
True},
+ {"name": "staging", "id": "C3", "is_private": False, "is_member":
True},
+ ]
+ mock_data = {"channels": channels, "response_metadata":
{"next_cursor": None}}
+
+ mock_response = MockResponse(mock_data)
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ result = get_channels_with_search(search_string="API", limit=100)
+
+ # Both "prod-api" and "dev-api" should match (case-insensitive)
+ assert len(result["result"]) == 2
+ assert {ch["name"] for ch in result["result"]} == {"prod-api",
"dev-api"}
+
+ def test_search_by_channel_id(self, mocker):
+ """Test search can match by channel ID"""
+ channels = [
+ {"name": "general", "id": "C12345", "is_private": False,
"is_member": True},
+ {"name": "random", "id": "C67890", "is_private": False,
"is_member": True},
+ ]
+ mock_data = {"channels": channels, "response_metadata":
{"next_cursor": None}}
+
+ mock_response = MockResponse(mock_data)
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ result = get_channels_with_search(
+ search_string="c12345", exact_match=True, limit=100
+ )
+
+ # Should match by ID (case-insensitive)
+ assert len(result["result"]) == 1
+ assert result["result"][0]["id"] == "C12345"
+
+ def test_non_search_empty_result_handling(self, mocker):
+ """Test non-search query handles empty channel list"""
+ mock_data = {
+ "channels": [],
+ "response_metadata": {"next_cursor": None},
+ }
+
+ mock_response = MockResponse(mock_data)
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ result = get_channels_with_search(limit=100)
+
+ assert len(result["result"]) == 0
+ assert result["has_more"] is False
+ assert result["next_cursor"] is None
+
+ def test_comma_separated_search_strings(self, mocker):
+ """Test search with comma-separated search strings (OR logic)"""
+ mock_data = {
+ "channels": [
+ {
+ "name": "engineering",
+ "id": "C1",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "marketing",
+ "id": "C2",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "sales",
+ "id": "C3",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "design",
+ "id": "C4",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "general",
+ "id": "C5",
+ "is_private": False,
+ "is_member": True,
+ },
+ ],
+ "response_metadata": {"next_cursor": None},
+ }
+
+ mock_response = MockResponse(mock_data)
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Search for "engineering,marketing,sales"
+ result = get_channels_with_search(
+ search_string="engineering,marketing,sales", limit=100
+ )
+
+ # Should match 3 channels: engineering, marketing, sales
+ assert len(result["result"]) == 3
+ channel_names = [channel["name"] for channel in result["result"]]
+ assert "engineering" in channel_names
+ assert "marketing" in channel_names
+ assert "sales" in channel_names
+ assert "design" not in channel_names
+ assert "general" not in channel_names
+
+ def test_comma_separated_search_with_whitespace(self, mocker):
+ """Test comma-separated search handles extra whitespace correctly"""
+ mock_data = {
+ "channels": [
+ {
+ "name": "engineering-team",
+ "id": "C1",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "marketing-ops",
+ "id": "C2",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "general",
+ "id": "C3",
+ "is_private": False,
+ "is_member": True,
+ },
+ ],
+ "response_metadata": {"next_cursor": None},
+ }
+
+ mock_response = MockResponse(mock_data)
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Search with extra whitespace: " engineering , marketing "
+ result = get_channels_with_search(
+ search_string=" engineering , marketing ", limit=100
+ )
+
+ # Should match 2 channels, whitespace should be stripped
+ assert len(result["result"]) == 2
+ channel_names = [channel["name"] for channel in result["result"]]
+ assert "engineering-team" in channel_names
+ assert "marketing-ops" in channel_names
+ assert "general" not in channel_names
+
+ def test_comma_separated_exact_match(self, mocker):
+ """Test comma-separated search with exact_match=True"""
+ mock_data = {
+ "channels": [
+ {
+ "name": "engineering",
+ "id": "C1",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "engineering-team",
+ "id": "C2",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "sales",
+ "id": "C3",
+ "is_private": False,
+ "is_member": True,
+ },
+ {
+ "name": "general",
+ "id": "C4",
+ "is_private": False,
+ "is_member": True,
+ },
+ ],
+ "response_metadata": {"next_cursor": None},
+ }
+
+ mock_response = MockResponse(mock_data)
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Exact match search for "engineering,sales"
+ result = get_channels_with_search(
+ search_string="engineering,sales", exact_match=True, limit=100
+ )
+
+ # Should match only exact names: engineering and sales (not
engineering-team)
+ assert len(result["result"]) == 2
+ channel_names = [channel["name"] for channel in result["result"]]
+ assert "engineering" in channel_names
+ assert "sales" in channel_names
+ assert "engineering-team" not in channel_names
+ assert "general" not in channel_names
+
+ def test_cache_boundary_exceeded_fallback_to_api(self, mocker):
+ """Test fallback to API when pagination exceeds cached data"""
+ # Mock cached channels (only 100 channels cached)
+ cached_channels = [
+ {
+ "name": f"channel-{i}",
+ "id": f"C{i}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(100)
+ ]
+
+ # Mock continuation channels from API
+ api_channels = [
+ {
+ "name": f"channel-{i}",
+ "id": f"C{i}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(100, 150)
+ ]
+
+ def cache_get_side_effect(key):
Review Comment:
**Suggestion:** Add type hints to this new helper function argument and
return type to satisfy the typing requirement. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This helper function has an untyped parameter and no return annotation, so
the type-hint rule applies here as well.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=527a10ea0b38473da173fb02a6ea438f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=527a10ea0b38473da173fb02a6ea438f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/slack_test.py
**Line:** 1350:1350
**Comment:**
*Custom Rule: Add type hints to this new helper function argument and
return type to satisfy the typing requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=f77f6f02ea5df0b562e7612ab4ff2b6b46f737bb7518df4b72479e6229907723&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=f77f6f02ea5df0b562e7612ab4ff2b6b46f737bb7518df4b72479e6229907723&reaction=dislike'>π</a>
##########
tests/unit_tests/utils/slack_test.py:
##########
@@ -520,3 +875,615 @@ def test_propagates_non_sdk_errors_from_probe(self,
mocker):
with pytest.raises(RuntimeError):
should_use_v2_api()
+
+
+class TestGetChannelsWithSearchPagination:
+ def test_partial_page_results(self, mocker):
+ """Test pagination with partial page (less than limit)"""
+
+ # Only 50 channels returned (less than default 100 limit)
+ channels = [
+ {
+ "name": f"channel-{i}",
+ "id": f"C{i}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(50)
+ ]
+
+ mock_data = {
+ "channels": channels,
+ "response_metadata": {"next_cursor": None},
+ }
+ mock_response = MockResponse(mock_data)
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ result = get_channels_with_search(limit=100)
+
+ # Should return all 50 channels
+ assert len(result["result"]) == 50
+ # Should indicate no more results
+ assert result["has_more"] is False
+ assert result["next_cursor"] is None
+
+ def test_streaming_search_stops_when_limit_reached(self, mocker):
+ """Test that streaming search stops immediately when limit is
reached"""
+
+ # First page with 60 matching channels
+ page1_channels = [
+ {"name": f"test-{i}", "id": f"C{i}", "is_private": False,
"is_member": True}
+ for i in range(60)
+ ]
+ # Second page with 60 more matching channels (should not be fully
processed)
+ page2_channels = [
+ {
+ "name": f"test-{i}",
+ "id": f"C{i + 60}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(60)
+ ]
+
+ mock_data_page1 = {
+ "channels": page1_channels,
+ "response_metadata": {"next_cursor": "page2"},
+ }
+ mock_data_page2 = {
+ "channels": page2_channels,
+ "response_metadata": {"next_cursor": "page3"},
+ }
+
+ mock_response_page1 = MockResponse(mock_data_page1)
+ mock_response_page2 = MockResponse(mock_data_page2)
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.side_effect = [
+ mock_response_page1,
+ mock_response_page2,
+ ]
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Request limit of 100
+ result = get_channels_with_search(search_string="test", limit=100)
+
+ # Should return exactly 100 channels (60 from page1 + 40 from page2)
+ assert len(result["result"]) == 100
+ # Should indicate more results available (next cursor points to page3)
+ assert result["has_more"] is True
+ assert result["next_cursor"] == "page3"
+
+ def test_cursor_format_with_special_characters(self, mocker):
+ """Test that cursor with special characters is handled correctly"""
+
+ # Slack cursors are base64 encoded strings that might contain special
chars
+ special_cursor = "dGVhbTpDMDYxRkE1UEw="
+
+ mock_data = {
+ "channels": [
+ {"name": "test", "id": "C123", "is_private": False,
"is_member": True},
+ ],
+ "response_metadata": {"next_cursor": None},
+ }
+ mock_response = MockResponse(mock_data)
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ # Call with special cursor
+ get_channels_with_search(cursor=special_cursor, limit=100)
+
+ # Verify cursor was passed to Slack API
+ mock_client.conversations_list.assert_called_once()
+ call_kwargs = mock_client.conversations_list.call_args[1]
+ assert call_kwargs["cursor"] == special_cursor
+
+ def test_empty_channel_list_response(self, mocker):
+ """Test handling of empty channels list from Slack API"""
+
+ mock_data = {
+ "channels": [],
+ "response_metadata": {"next_cursor": None},
+ }
+ mock_response = MockResponse(mock_data)
+
+ mock_client = mocker.Mock()
+ mock_client.conversations_list.return_value = mock_response
+ mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
+
+ result = get_channels_with_search()
+
+ assert result == {
+ "result": [],
+ "next_cursor": None,
+ "has_more": False,
+ }
+
+ def test_custom_limit_parameter(self, mocker):
+ """Test that custom limit parameter is respected"""
+
+ all_channels = [
+ {
+ "name": f"channel-{i}",
+ "id": f"C{i}",
+ "is_private": False,
+ "is_member": True,
+ }
+ for i in range(200)
+ ]
+
+ # Mock should respect the limit parameter (simulating Slack API
behavior)
+ def mock_conversations_list(**kwargs):
Review Comment:
**Suggestion:** Add parameter and return annotations to this nested helper
function (for example, typing `kwargs` and the returned mock response object).
[custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This added helper function omits annotations for `kwargs` and the return
type, so it violates the requirement to type-hint new Python functions where
possible.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0fb2b3e802794d908e6d73ccb71fc0f7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0fb2b3e802794d908e6d73ccb71fc0f7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/slack_test.py
**Line:** 1021:1021
**Comment:**
*Custom Rule: Add parameter and return annotations to this nested
helper function (for example, typing `kwargs` and the returned mock response
object).
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=35e0671daf8879012987a65c0347029720b5aca1283bcb2d2ce96796418f4a97&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=35e0671daf8879012987a65c0347029720b5aca1283bcb2d2ce96796418f4a97&reaction=dislike'>π</a>
##########
tests/unit_tests/tasks/test_slack.py:
##########
@@ -0,0 +1,134 @@
+# 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.
+
+from typing import Any, Optional
+
+from superset.tasks.slack import (
+ cache_channels,
+ DEFAULT_CACHE_WARMUP_TIME_LIMIT,
+ get_cache_warmup_options,
+)
+from superset.utils.slack import (
+ SLACK_CHANNELS_CACHE_KEY,
+ SLACK_CHANNELS_CONTINUATION_CURSOR_KEY,
+)
+
+
+def _page(ids: list[str], next_cursor: Optional[str], has_more: bool) ->
dict[str, Any]:
+ return {
+ "result": [{"id": channel_id} for channel_id in ids],
+ "next_cursor": next_cursor,
+ "has_more": has_more,
+ }
+
+
+def _patch_task(mocker, config, fetch_pages):
+ """Patch the task's collaborators and return the mocked cache."""
+ mock_app = mocker.patch("superset.tasks.slack.current_app")
+ mock_app.config = config
+ mock_fetch = mocker.patch(
+ "superset.tasks.slack._fetch_from_api", side_effect=fetch_pages
+ )
+ mock_cache = mocker.patch("superset.tasks.slack.cache_manager")
+ return mock_fetch, mock_cache
+
+
+def test_get_cache_warmup_options_uses_configured_timeout(mocker):
Review Comment:
**Suggestion:** Add a type hint for the test fixture parameter and annotate
the function return type. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The test function omits type hints for its fixture parameter and does not
declare a return type, so it matches the type-hint violation rule.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=037da5c29a9a4dc9b61209e3f54596e9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=037da5c29a9a4dc9b61209e3f54596e9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/tasks/test_slack.py
**Line:** 50:50
**Comment:**
*Custom Rule: Add a type hint for the test fixture parameter and
annotate the function return type.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=ee77d702993c6f06225d02bf9d1fe140a0c3d7aefc5d9daee80d1e4c28631315&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=ee77d702993c6f06225d02bf9d1fe140a0c3d7aefc5d9daee80d1e4c28631315&reaction=dislike'>π</a>
##########
tests/unit_tests/utils/slack_test.py:
##########
@@ -272,13 +493,84 @@ def
test_should_use_v2_api_passes_team_id_when_configured(self, mocker):
assert should_use_v2_api() is True
assert mock_client.conversations_list.call_args.kwargs["team_id"] ==
"T123456"
- def test_handle_pagination_multiple_pages(self, mocker):
+ def test_handle_pagination_without_search(self, mocker):
Review Comment:
**Suggestion:** Add type hints to this newly added method signature for the
fixture parameter and return type. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This newly added test method has no type annotations on the fixture
parameter or return type, so it matches the type-hint rule violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=05e5f9c55abd4e6b8b09676620eeb837&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=05e5f9c55abd4e6b8b09676620eeb837&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/slack_test.py
**Line:** 496:496
**Comment:**
*Custom Rule: Add type hints to this newly added method signature for
the fixture parameter and return type.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=aeec4550ed19ee54fcfacdd4b411ab76ce7d12cabbf4bffa0930233e70f210d0&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42074&comment_hash=aeec4550ed19ee54fcfacdd4b411ab76ce7d12cabbf4bffa0930233e70f210d0&reaction=dislike'>π</a>
--
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]
