codeant-ai-for-open-source[bot] commented on code in PR #43678:
URL: https://github.com/apache/superset/pull/43678#discussion_r3885529390
##########
superset/tasks/context.py:
##########
@@ -393,8 +393,14 @@ def set_cancellation(self, database_id: int,
cancel_query_id: str) -> None:
write. The orphan reaper reads it to cancel the query out-of-band when
this worker dies; the live abort path uses its in-memory closure
instead.
"""
- self._properties_cache["cancel_database_id"] = database_id
- self._properties_cache["cancel_query_id"] = cancel_query_id
+ self._properties_cache["private"] = cast(
+ "Any",
+ {
+ **(self._properties_cache.get("private") or {}),
+ "cancel_database_id": database_id,
+ "cancel_query_id": cancel_query_id,
+ },
+ )
Review Comment:
**Suggestion:** The cancellation-cache merge has the same unchecked mapping
assumption as the model helper. Since `_properties_cache` is initialized from
persisted free-form JSON, a non-dictionary `private` value causes
`set_cancellation()` to raise `TypeError` before the cancellation handle can be
recorded, which can make chart-data execution fail instead of merely disabling
orphan cleanup. Validate the existing bucket before unpacking it. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Async chart-data execution can fail during cancellation setup.
- ❌ Warehouse cancellation handles are not recorded.
- ⚠️ Orphan reaper cannot cancel abandoned queries.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=27149a04bae84d25862a204c4193eb77&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=27149a04bae84d25862a204c4193eb77&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tasks/context.py
**Line:** 396:403
**Comment:**
*Type Error: The cancellation-cache merge has the same unchecked
mapping assumption as the model helper. Since `_properties_cache` is
initialized from persisted free-form JSON, a non-dictionary `private` value
causes `set_cancellation()` to raise `TypeError` before the cancellation handle
can be recorded, which can make chart-data execution fail instead of merely
disabling orphan cleanup. Validate the existing bucket before unpacking it.
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%2F43678&comment_hash=263d68212724455f9c7b7b91dabad83a88e5e5297b7f3138952d59c06e2daabc&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43678&comment_hash=263d68212724455f9c7b7b91dabad83a88e5e5297b7f3138952d59c06e2daabc&reaction=dislike'>👎</a>
##########
superset/models/tasks.py:
##########
@@ -187,6 +187,22 @@ def update_properties(self, updates: TaskProperties) ->
None:
current.update(updates) # Merge updates
self.properties = serialize_properties(current)
+ def update_private_properties(self, updates: dict[str, Any]) -> None:
+ """
+ Merge keys into the ``private`` properties bucket (internal runtime
state).
+
+ ``private`` holds framework plumbing (job/cancel handles) that is never
+ surfaced to user-facing API payloads. Merges rather than replaces, so a
+ later write (e.g. the engine cancel handle) preserves an earlier one
(e.g.
+ the Celery job id).
+
+ :param updates: private keys to set/merge
+ """
+ current = cast(TaskProperties, dict(self.properties_dict))
+ private: dict[str, Any] = {**(current.get("private") or {}), **updates}
Review Comment:
**Suggestion:** The nested merge assumes the existing `private` value is a
mapping. Because task properties preserve arbitrary JSON values, a legacy or
externally supplied value such as a string, list, or number causes the
dictionary unpacking operation to raise `TypeError`, breaking any task update
that persists private state. Only merge when the existing value is a mapping,
or replace invalid values with an empty dictionary. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Task pickup can fail when private metadata is malformed.
- ❌ Celery job recovery metadata is not persisted.
- ⚠️ Orphan reaper cleanup can lose the job identifier.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=96b6f02e9b2e445abc997dce62236060&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=96b6f02e9b2e445abc997dce62236060&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/tasks.py
**Line:** 202:202
**Comment:**
*Type Error: The nested merge assumes the existing `private` value is a
mapping. Because task properties preserve arbitrary JSON values, a legacy or
externally supplied value such as a string, list, or number causes the
dictionary unpacking operation to raise `TypeError`, breaking any task update
that persists private state. Only merge when the existing value is a mapping,
or replace invalid values with an empty dictionary.
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%2F43678&comment_hash=4ba302cc1cde6490ba10b3fb90f4feebb19554c37acca2cb190fceae23b67e84&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43678&comment_hash=4ba302cc1cde6490ba10b3fb90f4feebb19554c37acca2cb190fceae23b67e84&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]
