codeant-ai-for-open-source[bot] commented on code in PR #34880:
URL: https://github.com/apache/superset/pull/34880#discussion_r3502568885
##########
superset/commands/dashboard/importers/v1/__init__.py:
##########
@@ -102,6 +128,10 @@ def _import(
if file_name.startswith("datasets/") and config["uuid"] in
dataset_uuids:
database_uuids.add(config["database_uuid"])
+ # assets inside dashboard databases, datasets and charts
+ # should be overwritten only if both flags are set to True
+ overwrite_assets = overwrite and kwargs.get("overwrite_all", False)
+
Review Comment:
**Suggestion:** Add an explicit type annotation for this new local flag
variable to satisfy typing requirements. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new local variable is inferred to be boolean and is introduced without
an explicit type annotation. The custom rule requires type hints for relevant
variables that can be annotated, so this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8bb2271859bb409a8f0ff637eaacaf6e&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=8bb2271859bb409a8f0ff637eaacaf6e&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/commands/dashboard/importers/v1/__init__.py
**Line:** 134:134
**Comment:**
*Custom Rule: Add an explicit type annotation for this new local flag
variable to satisfy typing requirements.
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%2F34880&comment_hash=5b2b3b346b2b7bd6a28782e32f02dbb03665c59387a7a183e46d9ad44b708283&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34880&comment_hash=5b2b3b346b2b7bd6a28782e32f02dbb03665c59387a7a183e46d9ad44b708283&reaction=dislike'>👎</a>
##########
superset/dashboards/api.py:
##########
@@ -1959,6 +1963,9 @@ def import_(self) -> Response:
else None
)
overwrite = request.form.get("overwrite") == "true"
+ overwrite_all = parse_boolean_string(
+ request.form.get("overwrite_all", "true" if overwrite else "false")
+ )
Review Comment:
**Suggestion:** Add an explicit boolean type annotation to the new local
variable assignment to satisfy the type-hint requirement. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The new local variable `overwrite_all` is introduced in modified Python code
without an explicit type annotation, and its type is clearly boolean, so this
violates the type-hint requirement.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8d1e4befc91245c7a969172978845d46&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=8d1e4befc91245c7a969172978845d46&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/dashboards/api.py
**Line:** 1966:1968
**Comment:**
*Custom Rule: Add an explicit boolean type annotation to the new local
variable assignment to satisfy the type-hint requirement.
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%2F34880&comment_hash=c172001aef4c8789cf23d554e83917df9f55fcab274fa5eb07814338290d148a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34880&comment_hash=c172001aef4c8789cf23d554e83917df9f55fcab274fa5eb07814338290d148a&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/importers/v1/assets_test.py:
##########
@@ -0,0 +1,200 @@
+# 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.
+
+import copy
+
+from pytest_mock import MockerFixture
+from sqlalchemy.orm.session import Session
+
+from tests.unit_tests.fixtures.assets_configs import (
+ charts_config_1,
+ dashboards_config_1,
+ databases_config,
+ datasets_config,
+)
+
+
+def test_import_dashboard_overwrite_charts_and_datasets(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Test that existing dashboards are overwritten.
+ """
+ import time
+
+ from superset import db, security_manager
+ from superset.commands.dashboard.importers.v1 import
ImportDashboardsCommand
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ engine = db.session.get_bind()
+ Dashboard.metadata.create_all(engine)
+
+ # for some reason it won't allow me to reuse the same config in the second
import
+ # thus declaring two configs with the same content
+ base_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
+ new_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
+
+ # gettings uuids
+ dasboard_configs = list(dashboards_config_1.values())
+ dashboard_uuid = dasboard_configs[0]["uuid"]
+ chart_configs = list(charts_config_1.values())
+ chart_uuid = chart_configs[0]["uuid"]
+ dataset_configs = list(datasets_config.values())
+ dataset_uuid = dataset_configs[0]["uuid"]
Review Comment:
**Suggestion:** Annotate these UUID local variables with an explicit string
or UUID type so the test data identifiers are typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
These local variables are new code and are inferable values that can be
annotated. Since they lack type hints, the suggestion matches the type-hint
requirement.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4914ca198a084734a766c711e6f84468&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=4914ca198a084734a766c711e6f84468&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/dashboard/importers/v1/assets_test.py
**Line:** 67:71
**Comment:**
*Custom Rule: Annotate these UUID local variables with an explicit
string or UUID type so the test data identifiers are 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%2F34880&comment_hash=c3a029d6b0a21acf59df60e7b514b9c3ff5714bea6894c9eda93296b6d3cc04b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34880&comment_hash=c3a029d6b0a21acf59df60e7b514b9c3ff5714bea6894c9eda93296b6d3cc04b&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/importers/v1/assets_test.py:
##########
@@ -0,0 +1,200 @@
+# 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.
+
+import copy
+
+from pytest_mock import MockerFixture
+from sqlalchemy.orm.session import Session
+
+from tests.unit_tests.fixtures.assets_configs import (
+ charts_config_1,
+ dashboards_config_1,
+ databases_config,
+ datasets_config,
+)
+
+
+def test_import_dashboard_overwrite_charts_and_datasets(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Test that existing dashboards are overwritten.
+ """
+ import time
+
+ from superset import db, security_manager
+ from superset.commands.dashboard.importers.v1 import
ImportDashboardsCommand
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ engine = db.session.get_bind()
+ Dashboard.metadata.create_all(engine)
+
+ # for some reason it won't allow me to reuse the same config in the second
import
+ # thus declaring two configs with the same content
+ base_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
+ new_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
+
+ # gettings uuids
+ dasboard_configs = list(dashboards_config_1.values())
+ dashboard_uuid = dasboard_configs[0]["uuid"]
+ chart_configs = list(charts_config_1.values())
+ chart_uuid = chart_configs[0]["uuid"]
+ dataset_configs = list(datasets_config.values())
+ dataset_uuid = dataset_configs[0]["uuid"]
+
+ # importing first time and retrieving initial records
+ ImportDashboardsCommand._import(base_configs, overwrite=True,
overwrite_all=True)
+ imported_dashboard = (
+ db.session.query(Dashboard).filter_by(uuid=dashboard_uuid).one()
+ )
+ imported_chart = db.session.query(Slice).filter_by(uuid=chart_uuid).one()
+ imported_dataset =
db.session.query(SqlaTable).filter_by(uuid=dataset_uuid).one()
+
+ # extracting changed_on field to compare later, ignoring milliseconds
+ initial_dashboard_changed_on = imported_dashboard.changed_on.strftime(
+ "%Y-%m-%d %H:%M:%S"
+ )
+ initial_chart_changed_on = imported_chart.changed_on.strftime("%Y-%m-%d
%H:%M:%S")
+ initial_dataset_changed_on = imported_dataset.changed_on.strftime(
+ "%Y-%m-%d %H:%M:%S"
+ )
+
+ # ensuring the changed_on field will be different
+ time.sleep(1)
+
+ # importing second time and retrieving updated records
+ ImportDashboardsCommand._import(new_configs, overwrite=True,
overwrite_all=True)
+ imported_dashboard = (
+ db.session.query(Dashboard).filter_by(uuid=dashboard_uuid).one()
+ )
+ imported_chart = db.session.query(Slice).filter_by(uuid=chart_uuid).one()
+ imported_dataset =
db.session.query(SqlaTable).filter_by(uuid=dataset_uuid).one()
+
+ # extracting changed_on field to compare with the previous retrieved
+ # ignoring milliseconds
+ final_dashboard_changed_on = imported_dashboard.changed_on.strftime(
+ "%Y-%m-%d %H:%M:%S"
+ )
+ final_chart_changed_on = imported_chart.changed_on.strftime("%Y-%m-%d
%H:%M:%S")
+ final_dataset_changed_on = imported_dataset.changed_on.strftime("%Y-%m-%d
%H:%M:%S")
+
+ # asserting the changed_on field was updated on all three records
+ assert initial_dashboard_changed_on != final_dashboard_changed_on
+ assert initial_chart_changed_on != final_chart_changed_on
+ assert initial_dataset_changed_on != final_dataset_changed_on
+ # asserting the changed_on field was updated to a later value on all three
records
+ assert final_dashboard_changed_on >= initial_dashboard_changed_on
+ assert final_chart_changed_on >= initial_chart_changed_on
+ assert final_dataset_changed_on >= initial_dataset_changed_on
+
+
+def test_import_dashboard_do_not_overwrite_charts_and_datasets(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Test that existing dashboards are overwritten but charts and datasets are
not.
+ """
+ import time
+
+ from superset import db, security_manager
+ from superset.commands.dashboard.importers.v1 import
ImportDashboardsCommand
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ engine = db.session.get_bind()
+ Dashboard.metadata.create_all(engine)
+
+ # for some reason it won't allow me to reuse the same config in the second
import
+ # thus declaring two configs with the same content
+ base_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
Review Comment:
**Suggestion:** Add an explicit type annotation for this configuration
mapping variable to comply with the required typing rule for relevant
variables. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is another newly added local dictionary in Python code with no type
annotation, so it violates the rule requiring type hints on relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ecb8b37782a943cfb55f7150261258b0&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=ecb8b37782a943cfb55f7150261258b0&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/dashboard/importers/v1/assets_test.py
**Line:** 140:145
**Comment:**
*Custom Rule: Add an explicit type annotation for this configuration
mapping variable to comply with the required typing rule for relevant 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%2F34880&comment_hash=2f7a92d869192afd8698e7b267e93da405c5ecd0beb065f635c6c4853d410cfc&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34880&comment_hash=2f7a92d869192afd8698e7b267e93da405c5ecd0beb065f635c6c4853d410cfc&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/importers/v1/assets_test.py:
##########
@@ -0,0 +1,200 @@
+# 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.
+
+import copy
+
+from pytest_mock import MockerFixture
+from sqlalchemy.orm.session import Session
+
+from tests.unit_tests.fixtures.assets_configs import (
+ charts_config_1,
+ dashboards_config_1,
+ databases_config,
+ datasets_config,
+)
+
+
+def test_import_dashboard_overwrite_charts_and_datasets(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Test that existing dashboards are overwritten.
+ """
+ import time
+
+ from superset import db, security_manager
+ from superset.commands.dashboard.importers.v1 import
ImportDashboardsCommand
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ engine = db.session.get_bind()
+ Dashboard.metadata.create_all(engine)
+
+ # for some reason it won't allow me to reuse the same config in the second
import
+ # thus declaring two configs with the same content
+ base_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
+ new_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
+
+ # gettings uuids
+ dasboard_configs = list(dashboards_config_1.values())
+ dashboard_uuid = dasboard_configs[0]["uuid"]
+ chart_configs = list(charts_config_1.values())
+ chart_uuid = chart_configs[0]["uuid"]
+ dataset_configs = list(datasets_config.values())
+ dataset_uuid = dataset_configs[0]["uuid"]
+
+ # importing first time and retrieving initial records
+ ImportDashboardsCommand._import(base_configs, overwrite=True,
overwrite_all=True)
+ imported_dashboard = (
+ db.session.query(Dashboard).filter_by(uuid=dashboard_uuid).one()
+ )
+ imported_chart = db.session.query(Slice).filter_by(uuid=chart_uuid).one()
+ imported_dataset =
db.session.query(SqlaTable).filter_by(uuid=dataset_uuid).one()
+
+ # extracting changed_on field to compare later, ignoring milliseconds
+ initial_dashboard_changed_on = imported_dashboard.changed_on.strftime(
+ "%Y-%m-%d %H:%M:%S"
+ )
+ initial_chart_changed_on = imported_chart.changed_on.strftime("%Y-%m-%d
%H:%M:%S")
+ initial_dataset_changed_on = imported_dataset.changed_on.strftime(
+ "%Y-%m-%d %H:%M:%S"
+ )
+
+ # ensuring the changed_on field will be different
+ time.sleep(1)
+
+ # importing second time and retrieving updated records
+ ImportDashboardsCommand._import(new_configs, overwrite=True,
overwrite_all=True)
+ imported_dashboard = (
+ db.session.query(Dashboard).filter_by(uuid=dashboard_uuid).one()
+ )
+ imported_chart = db.session.query(Slice).filter_by(uuid=chart_uuid).one()
+ imported_dataset =
db.session.query(SqlaTable).filter_by(uuid=dataset_uuid).one()
+
+ # extracting changed_on field to compare with the previous retrieved
+ # ignoring milliseconds
+ final_dashboard_changed_on = imported_dashboard.changed_on.strftime(
+ "%Y-%m-%d %H:%M:%S"
+ )
+ final_chart_changed_on = imported_chart.changed_on.strftime("%Y-%m-%d
%H:%M:%S")
+ final_dataset_changed_on = imported_dataset.changed_on.strftime("%Y-%m-%d
%H:%M:%S")
+
+ # asserting the changed_on field was updated on all three records
+ assert initial_dashboard_changed_on != final_dashboard_changed_on
+ assert initial_chart_changed_on != final_chart_changed_on
+ assert initial_dataset_changed_on != final_dataset_changed_on
+ # asserting the changed_on field was updated to a later value on all three
records
+ assert final_dashboard_changed_on >= initial_dashboard_changed_on
+ assert final_chart_changed_on >= initial_chart_changed_on
+ assert final_dataset_changed_on >= initial_dataset_changed_on
+
+
+def test_import_dashboard_do_not_overwrite_charts_and_datasets(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Test that existing dashboards are overwritten but charts and datasets are
not.
+ """
+ import time
+
+ from superset import db, security_manager
+ from superset.commands.dashboard.importers.v1 import
ImportDashboardsCommand
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ engine = db.session.get_bind()
+ Dashboard.metadata.create_all(engine)
+
+ # for some reason it won't allow me to reuse the same config in the second
import
+ # thus declaring two configs with the same content
+ base_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
+ new_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
+
+ # gettings uuids
+ dasboard_configs = list(dashboards_config_1.values())
+ dashboard_uuid = dasboard_configs[0]["uuid"]
+ chart_configs = list(charts_config_1.values())
+ chart_uuid = chart_configs[0]["uuid"]
+ dataset_configs = list(datasets_config.values())
+ dataset_uuid = dataset_configs[0]["uuid"]
Review Comment:
**Suggestion:** Annotate these UUID local variables with explicit types to
satisfy the type-hint rule for relevant locals. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
These are new local variables in the test and they are not annotated. The
custom rule explicitly flags missing type hints on relevant variables that can
be annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=256c23a99c6e4d5c81d882cbfaa853e2&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=256c23a99c6e4d5c81d882cbfaa853e2&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/dashboard/importers/v1/assets_test.py
**Line:** 155:159
**Comment:**
*Custom Rule: Annotate these UUID local variables with explicit types
to satisfy the type-hint rule for relevant locals.
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%2F34880&comment_hash=67f7dda01b3259fb203898dddf46f2bce96157670496025ec3d2fd08fe6f80dc&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34880&comment_hash=67f7dda01b3259fb203898dddf46f2bce96157670496025ec3d2fd08fe6f80dc&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/importers/v1/assets_test.py:
##########
@@ -0,0 +1,200 @@
+# 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.
+
+import copy
+
+from pytest_mock import MockerFixture
+from sqlalchemy.orm.session import Session
+
+from tests.unit_tests.fixtures.assets_configs import (
+ charts_config_1,
+ dashboards_config_1,
+ databases_config,
+ datasets_config,
+)
+
+
+def test_import_dashboard_overwrite_charts_and_datasets(
+ mocker: MockerFixture, session: Session
+) -> None:
+ """
+ Test that existing dashboards are overwritten.
+ """
+ import time
+
+ from superset import db, security_manager
+ from superset.commands.dashboard.importers.v1 import
ImportDashboardsCommand
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+
+ engine = db.session.get_bind()
+ Dashboard.metadata.create_all(engine)
+
+ # for some reason it won't allow me to reuse the same config in the second
import
+ # thus declaring two configs with the same content
+ base_configs = {
+ **copy.deepcopy(databases_config),
+ **copy.deepcopy(datasets_config),
+ **copy.deepcopy(charts_config_1),
+ **copy.deepcopy(dashboards_config_1),
+ }
Review Comment:
**Suggestion:** Add an explicit type annotation for this configuration
mapping variable to satisfy the type-hint requirement for relevant local
variables. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The rule requires type hints for relevant variables that can be annotated.
This local dictionary is newly added Python code and is unannotated, so the
suggestion correctly identifies a real typing omission.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0fb560256b294ad281d76a376337d7d3&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=0fb560256b294ad281d76a376337d7d3&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/dashboard/importers/v1/assets_test.py
**Line:** 52:57
**Comment:**
*Custom Rule: Add an explicit type annotation for this configuration
mapping variable to satisfy the type-hint requirement for relevant 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%2F34880&comment_hash=da6e9ffcbccfacfb9e06c30f50fbd03e4d99d045b815bd255d52c78bd2c11614&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34880&comment_hash=da6e9ffcbccfacfb9e06c30f50fbd03e4d99d045b815bd255d52c78bd2c11614&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]
