codeant-ai-for-open-source[bot] commented on code in PR #37516:
URL: https://github.com/apache/superset/pull/37516#discussion_r3579086804
##########
superset/charts/data/api.py:
##########
@@ -446,32 +484,40 @@ def _run_async(
# Clients will either poll or be notified of query completion,
# at which point they will call the /data/<cache_key> endpoint
# to retrieve the results.
- async_command = CreateAsyncChartDataJobCommand()
- try:
- async_command.validate(request)
- except AsyncQueryTokenException:
- return self.response_401()
+ with chart_timing_phase("enqueue"):
+ async_command = CreateAsyncChartDataJobCommand()
+ try:
+ async_command.validate(request)
+ except AsyncQueryTokenException:
+ return self.response_401()
- result = async_command.run(form_data, get_user_id())
- return self.response(202, **result)
+ async_result = async_command.run(form_data, get_user_id())
Review Comment:
**Suggestion:** Add an explicit type annotation for the async job response
variable to comply with the type-hint requirement for newly introduced local
variables. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This new local variable is introduced without an explicit type hint in
modified Python code, which matches the stated type-hint requirement for
annotatable 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=cea77632fdd24fd1a563a7db42ae2def&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=cea77632fdd24fd1a563a7db42ae2def&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/charts/data/api.py
**Line:** 494:494
**Comment:**
*Custom Rule: Add an explicit type annotation for the async job
response variable to comply with the type-hint requirement for newly introduced
local 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%2F37516&comment_hash=f2c2a1784a9b071615e0fe194a06e0c12195e8fb0bc6b621f0ce9d70a32f8305&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=f2c2a1784a9b071615e0fe194a06e0c12195e8fb0bc6b621f0ce9d70a32f8305&reaction=dislike'>๐</a>
##########
superset/charts/data/api.py:
##########
@@ -446,32 +484,40 @@ def _run_async(
# Clients will either poll or be notified of query completion,
# at which point they will call the /data/<cache_key> endpoint
# to retrieve the results.
- async_command = CreateAsyncChartDataJobCommand()
- try:
- async_command.validate(request)
- except AsyncQueryTokenException:
- return self.response_401()
+ with chart_timing_phase("enqueue"):
+ async_command = CreateAsyncChartDataJobCommand()
+ try:
+ async_command.validate(request)
+ except AsyncQueryTokenException:
+ return self.response_401()
- result = async_command.run(form_data, get_user_id())
- return self.response(202, **result)
+ async_result = async_command.run(form_data, get_user_id())
+ return self.response(202, **async_result)
def _send_chart_response( # noqa: C901
self,
- result: dict[Any, Any],
+ execution: ChartDataExecutionResult,
form_data: dict[str, Any] | None = None,
datasource: BaseDatasource | Query | None = None,
filename: str | None = None,
expected_rows: int | None = None,
dashboard_filter_context: DashboardFilterContext | None = None,
) -> Response:
+ result = execution.materialize()
Review Comment:
**Suggestion:** Add a concrete type annotation to the materialized execution
result variable introduced in this change. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a newly added local variable in modified code and it lacks an
explicit annotation, so it fits the custom 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=751cee5210524b8fbf840838fa8b74ff&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=751cee5210524b8fbf840838fa8b74ff&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/charts/data/api.py
**Line:** 506:506
**Comment:**
*Custom Rule: Add a concrete type annotation to the materialized
execution result variable introduced in this change.
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%2F37516&comment_hash=2de11f5f207de23d1fe996012a29e34241140411d66c83b9b17b16d593113729&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=2de11f5f207de23d1fe996012a29e34241140411d66c83b9b17b16d593113729&reaction=dislike'>๐</a>
##########
superset/charts/data/api.py:
##########
@@ -557,8 +604,10 @@ def _log_is_cached(
If there's a single query, logs the boolean value directly.
If multiple queries, logs as a list.
"""
- if add_extra_log_payload and result and "queries" in result:
- is_cached_values = [query.get("is_cached") for query in
result["queries"]]
+ if add_extra_log_payload:
+ is_cached_values = [
+ query.payload.get("is_cached") for query in result.queries
+ ]
Review Comment:
**Suggestion:** Annotate the newly added cached-status collection variable
with an explicit list type. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly introduced list variable is unannotated in the modified Python
code, so it violates the stated requirement to add type hints where applicable.
</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=c8615e483fd24b51b583b3382e1e7329&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=c8615e483fd24b51b583b3382e1e7329&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/charts/data/api.py
**Line:** 608:610
**Comment:**
*Custom Rule: Annotate the newly added cached-status collection
variable with an explicit list 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%2F37516&comment_hash=3e1db29167e00c1be376a3eac53bc9b1e5a03188d3a59c97f9a51eb9b11d9646&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=3e1db29167e00c1be376a3eac53bc9b1e5a03188d3a59c97f9a51eb9b11d9646&reaction=dislike'>๐</a>
##########
superset/common/query_context_processor.py:
##########
@@ -62,6 +92,27 @@
logger = logging.getLogger(__name__)
+_annotation_chart_stack: ContextVar[tuple[int, ...]] = ContextVar(
+ "chart_data_annotation_stack", default=()
+)
+
+_REUSABLE_CONTRIBUTION_RESULT_TYPES = frozenset(
+ {
+ ChartDataResultType.FULL,
+ ChartDataResultType.POST_PROCESSED,
+ ChartDataResultType.RESULTS,
+ }
+)
Review Comment:
**Suggestion:** Add an explicit type annotation to this new module-level
constant so static type checking can validate its intended element type.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a newly introduced module-level constant that can be annotated as a
frozenset of result types, but it currently has no explicit type hint. That
matches the Python 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=07808bee540d40c1a983fd1016fff217&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=07808bee540d40c1a983fd1016fff217&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/common/query_context_processor.py
**Line:** 99:105
**Comment:**
*Custom Rule: Add an explicit type annotation to this new module-level
constant so static type checking can validate its intended element 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%2F37516&comment_hash=709b46eb349e2e97c71220b7b3d247d7fcf23db2e1cdc6fc7c778efef34b90b5&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=709b46eb349e2e97c71220b7b3d247d7fcf23db2e1cdc6fc7c778efef34b90b5&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]
