codeant-ai-for-open-source[bot] commented on code in PR #42641:
URL: https://github.com/apache/superset/pull/42641#discussion_r3720534641
##########
superset/initialization/__init__.py:
##########
@@ -945,6 +984,40 @@ def _warn_if_retention_beat_missing(self) -> None:
"default CeleryConfig or add the entry to your override.",
self._RETENTION_TASK_NAME,
)
+ if self._RETENTION_TASK_MODULE not in imported_modules:
+ logger.warning(
+ "versioning: CELERY_CONFIG.imports is missing %r โ workers "
+ "will not register the retention task, so a scheduled run "
+ "fails with NotRegistered. Either inherit from the default "
+ "CeleryConfig or add the module to your override.",
+ self._RETENTION_TASK_MODULE,
+ )
+ # Resolve the gate exactly as the purge task does, so the warning
+ # cannot disagree with the no-op it is predicting.
+ # configure_feature_flags() runs before this check, so the manager
+ # is fully initialised here.
+ if feature_flag_manager.is_feature_enabled("SOFT_DELETE") and (
+ not beat_schedule or self._PURGE_TASK_NAME not in registered_tasks
+ ):
Review Comment:
**Suggestion:** The startup diagnostic is supposed to resolve `SOFT_DELETE`
from the static `DEFAULT_FEATURE_FLAGS` merged with `FEATURE_FLAGS`, but
`is_feature_enabled` may invoke the deployment-supplied
`GET_FEATURE_FLAGS_FUNC`. A dynamic, request-dependent, or otherwise overridden
result can therefore suppress or create a startup warning even though the
configured flag is enabled or disabled. Read the merged configuration-level
flags directly for this diagnostic rather than using the runtime feature-flag
resolver. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Startup diagnostic can miss missing purge schedules.
- โ ๏ธ Operators receive warnings unrelated to static configuration.
- โ ๏ธ Archived objects may accumulate without deploy-log visibility.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=66ef8e6966f3432cb9415da4632e4849&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=66ef8e6966f3432cb9415da4632e4849&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/initialization/__init__.py
**Line:** 999:1001
**Comment:**
*Incorrect Condition Logic: The startup diagnostic is supposed to
resolve `SOFT_DELETE` from the static `DEFAULT_FEATURE_FLAGS` merged with
`FEATURE_FLAGS`, but `is_feature_enabled` may invoke the deployment-supplied
`GET_FEATURE_FLAGS_FUNC`. A dynamic, request-dependent, or otherwise overridden
result can therefore suppress or create a startup warning even though the
configured flag is enabled or disabled. Read the merged configuration-level
flags directly for this diagnostic rather than using the runtime feature-flag
resolver.
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%2F42641&comment_hash=bd1d67460389b1f196bd50ddd38897a98e74d0ca4377791c45df51dcee7989e4&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42641&comment_hash=bd1d67460389b1f196bd50ddd38897a98e74d0ca4377791c45df51dcee7989e4&reaction=dislike'>๐</a>
##########
superset/initialization/__init__.py:
##########
@@ -945,6 +984,40 @@ def _warn_if_retention_beat_missing(self) -> None:
"default CeleryConfig or add the entry to your override.",
self._RETENTION_TASK_NAME,
)
+ if self._RETENTION_TASK_MODULE not in imported_modules:
+ logger.warning(
+ "versioning: CELERY_CONFIG.imports is missing %r โ workers "
+ "will not register the retention task, so a scheduled run "
+ "fails with NotRegistered. Either inherit from the default "
+ "CeleryConfig or add the module to your override.",
+ self._RETENTION_TASK_MODULE,
+ )
Review Comment:
**Suggestion:** `CELERY_CONFIG.imports` is not the authoritative set of
modules registered with a worker: Celery can load task modules through
`include`, autodiscovery, worker command-line imports, or imports performed
elsewhere during worker startup. A deployment using one of those mechanisms
will have the task registered successfully but will still receive an
actionable-looking missing-import warning. Check the actual Celery task
registry after configuration, or limit this diagnostic to configurations where
`imports` is explicitly the registration mechanism. [api mismatch]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Valid Celery deployments receive false startup warnings.
- โ ๏ธ Operators may incorrectly modify working worker configuration.
- โ ๏ธ Diagnostic trust is reduced for retention setup.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c1e9d6c05e8d4c0da909457e2ae4f332&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=c1e9d6c05e8d4c0da909457e2ae4f332&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/initialization/__init__.py
**Line:** 987:994
**Comment:**
*Api Mismatch: `CELERY_CONFIG.imports` is not the authoritative set of
modules registered with a worker: Celery can load task modules through
`include`, autodiscovery, worker command-line imports, or imports performed
elsewhere during worker startup. A deployment using one of those mechanisms
will have the task registered successfully but will still receive an
actionable-looking missing-import warning. Check the actual Celery task
registry after configuration, or limit this diagnostic to configurations where
`imports` is explicitly the registration mechanism.
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%2F42641&comment_hash=1c92a5b0e09b523e65c1a6d30eb1959bc15fe4ce7d4822a11b30f7387a0ccb42&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42641&comment_hash=1c92a5b0e09b523e65c1a6d30eb1959bc15fe4ce7d4822a11b30f7387a0ccb42&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]
