codeant-ai-for-open-source[bot] commented on code in PR #41580:
URL: https://github.com/apache/superset/pull/41580#discussion_r3503460210
##########
tests/unit_tests/commands/datasource/list_test.py:
##########
@@ -19,38 +19,27 @@
import pytest
from sqlalchemy import literal, select
-from superset.commands.datasource.list import GetCombinedDatasourceListCommand
+from superset.commands.datasource.list import (
+ _apply_owners_filter,
+ GetCombinedDatasourceListCommand,
+)
def test_parse_filters_semantic_view_requires_dataset_operator() -> None:
- (
- source_type,
- name_filter,
- sql_filter,
- type_filter,
- database_id,
- semantic_layer_uuid,
- ) = GetCombinedDatasourceListCommand._parse_filters(
+ filters = GetCombinedDatasourceListCommand._parse_filters(
[{"col": "sql", "opr": "eq", "value": "semantic_view"}]
)
Review Comment:
**Suggestion:** Add an explicit type annotation for the local `filters`
variable to satisfy the type-hint requirement for newly introduced variables.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new local variable is assigned without any type annotation in modified
Python code, which matches the rule requiring type hints on annotatable
variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=73224ff9d814472db6557ad514906a40&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=73224ff9d814472db6557ad514906a40&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/commands/datasource/list_test.py
**Line:** 29:31
**Comment:**
*Custom Rule: Add an explicit type annotation for the local `filters`
variable to satisfy the type-hint requirement for newly introduced 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%2F41580&comment_hash=affec7eb464967fd5e79e0bc63cc725315e3e794d8271d70a7eb74cca9b94d42&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41580&comment_hash=affec7eb464967fd5e79e0bc63cc725315e3e794d8271d70a7eb74cca9b94d42&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/datasource/list_test.py:
##########
@@ -60,32 +49,91 @@ def
test_parse_filters_semantic_view_with_dataset_operator() -> None:
]
)
Review Comment:
**Suggestion:** Add a type hint to the `filters` assignment in this new test
so the variable is explicitly typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This added assignment introduces an untyped local variable in Python code
that can be annotated, so it violates the type-hint requirement.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4a81922fa91d464d8fe57283b239f143&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=4a81922fa91d464d8fe57283b239f143&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/commands/datasource/list_test.py
**Line:** 42:50
**Comment:**
*Custom Rule: Add a type hint to the `filters` assignment in this new
test so the variable is 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%2F41580&comment_hash=6f8b8e80a0449dcc95e78a6b29c290f089b61374d442650258bafd3b7a534af8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41580&comment_hash=6f8b8e80a0449dcc95e78a6b29c290f089b61374d442650258bafd3b7a534af8&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/datasource/list_test.py:
##########
@@ -60,32 +49,91 @@ def
test_parse_filters_semantic_view_with_dataset_operator() -> None:
]
)
- assert source_type == "all"
- assert name_filter is None
- assert sql_filter is None
- assert type_filter == "semantic_view"
- assert database_id is None
- assert semantic_layer_uuid is None
+ assert filters.source_type == "all"
+ assert filters.name_filter is None
+ assert filters.sql_filter is None
+ assert filters.type_filter == "semantic_view"
+ assert filters.database_id is None
+ assert filters.semantic_layer_uuid is None
def test_parse_filters_sql_bool_requires_dataset_operator() -> None:
- (
- source_type,
- name_filter,
- sql_filter,
- type_filter,
- database_id,
- semantic_layer_uuid,
- ) = GetCombinedDatasourceListCommand._parse_filters(
+ filters = GetCombinedDatasourceListCommand._parse_filters(
[{"col": "sql", "opr": "eq", "value": True}]
)
- assert source_type == "all"
- assert name_filter is None
- assert sql_filter is None
- assert type_filter is None
- assert database_id is None
- assert semantic_layer_uuid is None
+ assert filters.source_type == "all"
+ assert filters.name_filter is None
+ assert filters.sql_filter is None
+ assert filters.type_filter is None
+ assert filters.database_id is None
+ assert filters.semantic_layer_uuid is None
+
+
+def test_parse_filters_schema() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "schema", "opr": "eq", "value": "public"}]
+ )
Review Comment:
**Suggestion:** Provide an explicit type annotation for the local `filters`
variable introduced in this test case. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The modified test introduces another unannotated local variable, which is
covered by the Python type-hint rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=40819bf8b6fa4cbdacd8ceb477ce1344&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=40819bf8b6fa4cbdacd8ceb477ce1344&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/commands/datasource/list_test.py
**Line:** 74:76
**Comment:**
*Custom Rule: Provide an explicit type annotation for the local
`filters` variable introduced in this test case.
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%2F41580&comment_hash=551e6b24f8c37f7ccedaf0a660b445b1b1259f41629534bb9b6afd93b54c6b25&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41580&comment_hash=551e6b24f8c37f7ccedaf0a660b445b1b1259f41629534bb9b6afd93b54c6b25&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/datasource/list_test.py:
##########
@@ -60,32 +49,91 @@ def
test_parse_filters_semantic_view_with_dataset_operator() -> None:
]
)
- assert source_type == "all"
- assert name_filter is None
- assert sql_filter is None
- assert type_filter == "semantic_view"
- assert database_id is None
- assert semantic_layer_uuid is None
+ assert filters.source_type == "all"
+ assert filters.name_filter is None
+ assert filters.sql_filter is None
+ assert filters.type_filter == "semantic_view"
+ assert filters.database_id is None
+ assert filters.semantic_layer_uuid is None
def test_parse_filters_sql_bool_requires_dataset_operator() -> None:
- (
- source_type,
- name_filter,
- sql_filter,
- type_filter,
- database_id,
- semantic_layer_uuid,
- ) = GetCombinedDatasourceListCommand._parse_filters(
+ filters = GetCombinedDatasourceListCommand._parse_filters(
[{"col": "sql", "opr": "eq", "value": True}]
)
- assert source_type == "all"
- assert name_filter is None
- assert sql_filter is None
- assert type_filter is None
- assert database_id is None
- assert semantic_layer_uuid is None
+ assert filters.source_type == "all"
+ assert filters.name_filter is None
+ assert filters.sql_filter is None
+ assert filters.type_filter is None
+ assert filters.database_id is None
+ assert filters.semantic_layer_uuid is None
+
+
+def test_parse_filters_schema() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "schema", "opr": "eq", "value": "public"}]
+ )
+
+ assert filters.schema_filter == "public"
+ assert filters.database_id is None
+ assert filters.owners_filter is None
+
+
+def test_parse_filters_owners_scalar() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "owners", "opr": "rel_m_m", "value": "5"}]
+ )
+
+ assert filters.owners_filter == [5]
+ assert filters.schema_filter is None
+ assert filters.changed_by_filter is None
+
+
+def test_parse_filters_owners_list() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "owners", "opr": "rel_m_m", "value": ["5", "7"]}]
+ )
+
+ assert filters.owners_filter == [5, 7]
+
+
+def test_parse_filters_owners_none() -> None:
+ assert _apply_owners_filter(None) is None
+
+
+def test_parse_filters_changed_by() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "changed_by", "opr": "rel_o_m", "value": "3"}]
+ )
+
+ assert filters.changed_by_filter == 3
+ assert filters.owners_filter is None
+ assert filters.schema_filter is None
+
+
+def test_parse_filters_certified_true() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "id", "opr": "dataset_is_certified", "value": True}]
+ )
+
+ assert filters.certified_filter is True
+
+
+def test_parse_filters_certified_false() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "id", "opr": "dataset_is_certified", "value": False}]
+ )
+
+ assert filters.certified_filter is False
+
+
+def test_parse_filters_certified_non_bool_ignored() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "id", "opr": "dataset_is_certified", "value": "false"}]
+ )
Review Comment:
**Suggestion:** Annotate the `filters` variable with its expected type in
this newly added test to keep type hints complete. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This added assignment lacks an explicit type hint, and the variable is
trivially annotatable, so it is a real violation of the type-hint rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e83630c899cc4223a9e94453de224226&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=e83630c899cc4223a9e94453de224226&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/commands/datasource/list_test.py
**Line:** 132:134
**Comment:**
*Custom Rule: Annotate the `filters` variable with its expected type in
this newly added test to keep type hints complete.
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%2F41580&comment_hash=d7ecfddd29358d12ab32a1087af08e1caade9b77f117b6c0c54e232e7f170055&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41580&comment_hash=d7ecfddd29358d12ab32a1087af08e1caade9b77f117b6c0c54e232e7f170055&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/datasource/list_test.py:
##########
@@ -60,32 +49,91 @@ def
test_parse_filters_semantic_view_with_dataset_operator() -> None:
]
)
- assert source_type == "all"
- assert name_filter is None
- assert sql_filter is None
- assert type_filter == "semantic_view"
- assert database_id is None
- assert semantic_layer_uuid is None
+ assert filters.source_type == "all"
+ assert filters.name_filter is None
+ assert filters.sql_filter is None
+ assert filters.type_filter == "semantic_view"
+ assert filters.database_id is None
+ assert filters.semantic_layer_uuid is None
def test_parse_filters_sql_bool_requires_dataset_operator() -> None:
- (
- source_type,
- name_filter,
- sql_filter,
- type_filter,
- database_id,
- semantic_layer_uuid,
- ) = GetCombinedDatasourceListCommand._parse_filters(
+ filters = GetCombinedDatasourceListCommand._parse_filters(
[{"col": "sql", "opr": "eq", "value": True}]
)
- assert source_type == "all"
- assert name_filter is None
- assert sql_filter is None
- assert type_filter is None
- assert database_id is None
- assert semantic_layer_uuid is None
+ assert filters.source_type == "all"
+ assert filters.name_filter is None
+ assert filters.sql_filter is None
+ assert filters.type_filter is None
+ assert filters.database_id is None
+ assert filters.semantic_layer_uuid is None
+
+
+def test_parse_filters_schema() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "schema", "opr": "eq", "value": "public"}]
+ )
+
+ assert filters.schema_filter == "public"
+ assert filters.database_id is None
+ assert filters.owners_filter is None
+
+
+def test_parse_filters_owners_scalar() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "owners", "opr": "rel_m_m", "value": "5"}]
+ )
+
+ assert filters.owners_filter == [5]
+ assert filters.schema_filter is None
+ assert filters.changed_by_filter is None
+
+
+def test_parse_filters_owners_list() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "owners", "opr": "rel_m_m", "value": ["5", "7"]}]
+ )
+
+ assert filters.owners_filter == [5, 7]
+
+
+def test_parse_filters_owners_none() -> None:
+ assert _apply_owners_filter(None) is None
+
+
+def test_parse_filters_changed_by() -> None:
+ filters = GetCombinedDatasourceListCommand._parse_filters(
+ [{"col": "changed_by", "opr": "rel_o_m", "value": "3"}]
+ )
Review Comment:
**Suggestion:** Add a concrete type hint for `filters` in this new block to
comply with the Python type-hint rule. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly introduced Python local variable without a type annotation,
so the rule applies directly.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8d6bcc17b92f496a8a54e912d636dbfe&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=8d6bcc17b92f496a8a54e912d636dbfe&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/commands/datasource/list_test.py
**Line:** 106:108
**Comment:**
*Custom Rule: Add a concrete type hint for `filters` in this new block
to comply with the Python type-hint 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%2F41580&comment_hash=565a23cf476c158c0bd4efeb9bb8a7cd4d665a3d443ae21dac78d4dbf1872ba8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41580&comment_hash=565a23cf476c158c0bd4efeb9bb8a7cd4d665a3d443ae21dac78d4dbf1872ba8&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]
