codeant-ai-for-open-source[bot] commented on code in PR #41509:
URL: https://github.com/apache/superset/pull/41509#discussion_r3531517442
##########
superset/views/datasource/views.py:
##########
@@ -208,6 +208,23 @@ def samples(self) -> FlaskResponse:
payload = SamplesPayloadSchema().load(request.json)
except ValidationError as err:
return json_error_response(err.messages, status=400)
+
+ # Refuse early for datasource types that don't model raw rows
+ # (e.g. semantic views, which only expose pre-defined metrics and
+ # dimensions). Without this gate the request would still go through
+ # the standard query pipeline and fail with an opaque 500.
+ # ``supports_samples`` defaults to True for any datasource class that
+ # doesn't explicitly opt out, so SqlaTable/Query/SavedQuery continue
+ # to work without needing the attribute declared on each class.
+ ds_class = DatasourceDAO.sources.get(
+ DatasourceType(params["datasource_type"]),
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced
local variable to comply with the type-hinting rule for annotatable variables.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The new local variable `ds_class` is introduced without a type annotation
even though it is a Python variable that can be annotated. This matches the
custom rule requiring type hints for new or modified annotatable variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b0d1a7dc73e54b79a3dbfdb546ceb3e9&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=b0d1a7dc73e54b79a3dbfdb546ceb3e9&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/views/datasource/views.py
**Line:** 219:221
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced
local variable to comply with the type-hinting rule for annotatable 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%2F41509&comment_hash=d36c88959e286bfe95ecaace14505ab5975952411267cb296772b0545176c153&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41509&comment_hash=d36c88959e286bfe95ecaace14505ab5975952411267cb296772b0545176c153&reaction=dislike'>👎</a>
##########
tests/unit_tests/common/test_query_actions_drill_detail.py:
##########
@@ -0,0 +1,70 @@
+# 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 unittest.mock import MagicMock, patch
+
+import pytest
+
+from superset.common.query_actions import _get_drill_detail
+from superset.exceptions import QueryObjectValidationError
+
+
+def test_get_drill_detail_refuses_datasource_that_opts_out() -> None:
+ """
+ A datasource with ``supports_drill_to_detail = False`` (e.g. semantic
+ views) must be hard-blocked on the server. Without this gate the request
+ would fall through to ``_get_full`` and fail with an opaque error, and
+ the flag would only be enforced by the frontend menu — leaving the
+ chart-data API endpoint accepting drill-detail requests it shouldn't.
+ """
+ datasource = MagicMock()
+ datasource.supports_drill_to_detail = False
+
+ query_obj = MagicMock()
+ query_obj.datasource = datasource
+
+ query_context = MagicMock()
+
Review Comment:
**Suggestion:** Add explicit type annotations for the local mock variables
in this test so annotatable variables are not left untyped. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The rule requires type hints on new Python variables that can reasonably be
annotated. These local mock variables are newly introduced and unannotated, so
the suggestion identifies a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2bbd53f4f596434ba8ca9ac6f96ab58c&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=2bbd53f4f596434ba8ca9ac6f96ab58c&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/common/test_query_actions_drill_detail.py
**Line:** 33:40
**Comment:**
*Custom Rule: Add explicit type annotations for the local mock
variables in this test so annotatable variables are not left untyped.
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%2F41509&comment_hash=6a74a9d6b628cd9a4c5d14684a6087694bf0c912ea810af2f29c6d2b24831108&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41509&comment_hash=6a74a9d6b628cd9a4c5d14684a6087694bf0c912ea810af2f29c6d2b24831108&reaction=dislike'>👎</a>
##########
tests/unit_tests/common/test_query_actions_drill_detail.py:
##########
@@ -0,0 +1,70 @@
+# 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 unittest.mock import MagicMock, patch
+
+import pytest
+
+from superset.common.query_actions import _get_drill_detail
+from superset.exceptions import QueryObjectValidationError
+
+
+def test_get_drill_detail_refuses_datasource_that_opts_out() -> None:
+ """
+ A datasource with ``supports_drill_to_detail = False`` (e.g. semantic
+ views) must be hard-blocked on the server. Without this gate the request
+ would fall through to ``_get_full`` and fail with an opaque error, and
+ the flag would only be enforced by the frontend menu — leaving the
+ chart-data API endpoint accepting drill-detail requests it shouldn't.
+ """
+ datasource = MagicMock()
+ datasource.supports_drill_to_detail = False
+
+ query_obj = MagicMock()
+ query_obj.datasource = datasource
+
+ query_context = MagicMock()
+
+ with pytest.raises(
+ QueryObjectValidationError,
+ match="Drill to detail is not available",
+ ):
+ _get_drill_detail(query_context, query_obj)
+
+
+def test_get_drill_detail_allows_datasource_without_flag() -> None:
+ """
+ Datasources that don't declare the flag (e.g. legacy ``SqlaTable``
+ subclasses via ``getattr`` default) must continue to work — the gate
+ only fires when the flag is explicitly ``False``.
+ """
+ datasource = MagicMock(spec=["columns"])
+ column = MagicMock()
+ column.column_name = "id"
+ datasource.columns = [column]
+
+ query_obj = MagicMock()
+ query_obj.datasource = datasource
+ query_obj.columns = []
+
+ query_context = MagicMock()
+
Review Comment:
**Suggestion:** Add explicit type annotations for these local variables to
comply with the requirement that relevant new variables include type hints.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
These are newly added local variables in Python code and the custom rule
explicitly calls for type hints on relevant variables that can be annotated.
Since they are left untyped, the suggestion matches a real rule violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=342d233034f14e7485bad33e90a8ce20&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=342d233034f14e7485bad33e90a8ce20&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/common/test_query_actions_drill_detail.py
**Line:** 54:64
**Comment:**
*Custom Rule: Add explicit type annotations for these local variables
to comply with the requirement that relevant new variables include type hints.
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%2F41509&comment_hash=9b86dcdd6c994d38cb716e48132201f71eee983b8fcc931340dcb58be192a0e9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41509&comment_hash=9b86dcdd6c994d38cb716e48132201f71eee983b8fcc931340dcb58be192a0e9&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]
