codeant-ai-for-open-source[bot] commented on code in PR #41076:
URL: https://github.com/apache/superset/pull/41076#discussion_r3500130747
##########
superset/daos/dataset.py:
##########
@@ -275,6 +275,103 @@ def update(
return super().update(item, attributes)
+ @classmethod
+ def _validate_column_date_formats(
+ cls, property_columns: list[dict[str, Any]]
+ ) -> None:
+ for column in property_columns:
+ if column.get("python_date_format") is None:
+ continue
+ if not
DatasetDAO.validate_python_date_format(column["python_date_format"]):
+ raise ValueError(
+ "python_date_format is an invalid date/timestamp format."
+ )
+
+ @classmethod
+ def _override_columns(
+ cls, model: SqlaTable, property_columns: list[dict[str, Any]]
+ ) -> None:
+ """Replace columns by natural key (``column_name``) β update in place
+ rather than delete-and-reinsert.
+
+ SPIKE (full-Continuum): the previous
+ delete-and-reinsert pattern produced overlapping shadow rows in
+ ``table_columns_version`` (the same ``column_name`` had a DELETE
+ shadow at tx N alongside an INSERT shadow at tx N for a fresh PK).
+ Continuum's ``Reverter`` couldn't unwind this on restore: its flush
+ ordering inserts the historical row before deleting the live one,
+ hitting the ``UNIQUE (table_id, column_name)`` constraint mid-flush
+ (ADR-004 Failure 1).
+
+ The natural-key upsert keeps PKs stable across metadata refresh.
+ Continuum captures only real field changes; new columns get plain
+ INSERT shadows; removed columns get plain DELETE shadows. No
+ natural-key collisions, so Reverter can restore cleanly.
+
+ Behaviour change vs. the previous implementation: PKs of unchanged
+ columns are preserved. Charts that reference columns by their
+ ``id`` continue to work across a metadata refresh β previously
+ such references would be invalidated.
+ """
+ existing_by_name = {c.column_name: c for c in model.columns}
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced
lookup map to satisfy the type-hinting rule for relevant variables.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly added local variable in modified Python code, and it is an
annotatable lookup map without an explicit type hint, so it matches the
type-hint rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=13a59401e58c46c69691c2f5760a8ff6&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=13a59401e58c46c69691c2f5760a8ff6&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/daos/dataset.py
**Line:** 316:316
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced
lookup map to satisfy the type-hinting 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%2F41076&comment_hash=b9ea6687955bd858c3018845e376d36e28afa86524492c94b23bbab4baab710c&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=b9ea6687955bd858c3018845e376d36e28afa86524492c94b23bbab4baab710c&reaction=dislike'>π</a>
##########
superset/daos/dataset.py:
##########
@@ -275,6 +275,103 @@ def update(
return super().update(item, attributes)
+ @classmethod
+ def _validate_column_date_formats(
+ cls, property_columns: list[dict[str, Any]]
+ ) -> None:
+ for column in property_columns:
+ if column.get("python_date_format") is None:
+ continue
+ if not
DatasetDAO.validate_python_date_format(column["python_date_format"]):
+ raise ValueError(
+ "python_date_format is an invalid date/timestamp format."
+ )
+
+ @classmethod
+ def _override_columns(
+ cls, model: SqlaTable, property_columns: list[dict[str, Any]]
+ ) -> None:
+ """Replace columns by natural key (``column_name``) β update in place
+ rather than delete-and-reinsert.
+
+ SPIKE (full-Continuum): the previous
+ delete-and-reinsert pattern produced overlapping shadow rows in
+ ``table_columns_version`` (the same ``column_name`` had a DELETE
+ shadow at tx N alongside an INSERT shadow at tx N for a fresh PK).
+ Continuum's ``Reverter`` couldn't unwind this on restore: its flush
+ ordering inserts the historical row before deleting the live one,
+ hitting the ``UNIQUE (table_id, column_name)`` constraint mid-flush
+ (ADR-004 Failure 1).
+
+ The natural-key upsert keeps PKs stable across metadata refresh.
+ Continuum captures only real field changes; new columns get plain
+ INSERT shadows; removed columns get plain DELETE shadows. No
+ natural-key collisions, so Reverter can restore cleanly.
+
+ Behaviour change vs. the previous implementation: PKs of unchanged
+ columns are preserved. Charts that reference columns by their
+ ``id`` continue to work across a metadata refresh β previously
+ such references would be invalidated.
+ """
+ existing_by_name = {c.column_name: c for c in model.columns}
+ incoming_by_name = {p["column_name"]: p for p in property_columns}
+
+ # Identity is the natural key here, never the payload's ``id``:
+ # setattr-ing an incoming ``id`` onto a name-matched row would
+ # rewrite a live primary key, and a renamed column whose payload
+ # still carries its old ``id`` would INSERT with a live PK while
+ # the old-named row is deleted in the same flush β INSERTs flush
+ # before DELETEs, so that collides on the PK / UNIQUE(table_id,
+ # column_name) constraints. ``table_id`` is pinned to *model*.
+ protected_keys = ("id", "table_id")
Review Comment:
**Suggestion:** Add a type annotation for this new tuple constant so its
expected element type is explicit. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a new tuple constant in modified code and it lacks an explicit type
annotation, which is exactly what the rule flags for relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=edbde84e52134cbe8327371a11f3c364&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=edbde84e52134cbe8327371a11f3c364&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/daos/dataset.py
**Line:** 326:326
**Comment:**
*Custom Rule: Add a type annotation for this new tuple constant so its
expected element type is explicit.
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%2F41076&comment_hash=b21128a5ede41b69612339875d56c13155248805014b721f89bf5e94fe3c0485&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=b21128a5ede41b69612339875d56c13155248805014b721f89bf5e94fe3c0485&reaction=dislike'>π</a>
##########
superset/daos/dataset.py:
##########
@@ -275,6 +275,103 @@ def update(
return super().update(item, attributes)
+ @classmethod
+ def _validate_column_date_formats(
+ cls, property_columns: list[dict[str, Any]]
+ ) -> None:
+ for column in property_columns:
+ if column.get("python_date_format") is None:
+ continue
+ if not
DatasetDAO.validate_python_date_format(column["python_date_format"]):
+ raise ValueError(
+ "python_date_format is an invalid date/timestamp format."
+ )
+
+ @classmethod
+ def _override_columns(
+ cls, model: SqlaTable, property_columns: list[dict[str, Any]]
+ ) -> None:
+ """Replace columns by natural key (``column_name``) β update in place
+ rather than delete-and-reinsert.
+
+ SPIKE (full-Continuum): the previous
+ delete-and-reinsert pattern produced overlapping shadow rows in
+ ``table_columns_version`` (the same ``column_name`` had a DELETE
+ shadow at tx N alongside an INSERT shadow at tx N for a fresh PK).
+ Continuum's ``Reverter`` couldn't unwind this on restore: its flush
+ ordering inserts the historical row before deleting the live one,
+ hitting the ``UNIQUE (table_id, column_name)`` constraint mid-flush
+ (ADR-004 Failure 1).
+
+ The natural-key upsert keeps PKs stable across metadata refresh.
+ Continuum captures only real field changes; new columns get plain
+ INSERT shadows; removed columns get plain DELETE shadows. No
+ natural-key collisions, so Reverter can restore cleanly.
+
+ Behaviour change vs. the previous implementation: PKs of unchanged
+ columns are preserved. Charts that reference columns by their
+ ``id`` continue to work across a metadata refresh β previously
+ such references would be invalidated.
+ """
+ existing_by_name = {c.column_name: c for c in model.columns}
+ incoming_by_name = {p["column_name"]: p for p in property_columns}
+
+ # Identity is the natural key here, never the payload's ``id``:
+ # setattr-ing an incoming ``id`` onto a name-matched row would
+ # rewrite a live primary key, and a renamed column whose payload
+ # still carries its old ``id`` would INSERT with a live PK while
+ # the old-named row is deleted in the same flush β INSERTs flush
+ # before DELETEs, so that collides on the PK / UNIQUE(table_id,
+ # column_name) constraints. ``table_id`` is pinned to *model*.
+ protected_keys = ("id", "table_id")
+
+ # Update columns present in both: in-place setattr.
+ for name, col in existing_by_name.items():
+ if name in incoming_by_name:
+ for key, value in incoming_by_name[name].items():
+ if key not in protected_keys:
+ setattr(col, key, value)
+
+ # Insert columns present only in incoming.
+ for name, properties in incoming_by_name.items():
+ if name not in existing_by_name:
+ cleaned = {
+ key: value
+ for key, value in properties.items()
+ if key not in protected_keys
+ }
+ db.session.add(TableColumn(**{**cleaned, "table_id":
model.id}))
+
+ # Delete columns present only in existing.
+ for name, col in existing_by_name.items():
+ if name not in incoming_by_name:
+ db.session.delete(col)
+
+ @classmethod
+ def _upsert_columns(
+ cls, model: SqlaTable, property_columns: list[dict[str, Any]]
+ ) -> None:
+ columns_by_id = {column.id: column for column in model.columns}
Review Comment:
**Suggestion:** Add a concrete type annotation for this new ID-indexed map
variable. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly introduced dictionary used as an ID lookup map and it has no
explicit type annotation, so it is a real instance of the type-hint omission
rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8070461118f0465a8e816d24259443c4&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=8070461118f0465a8e816d24259443c4&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/daos/dataset.py
**Line:** 354:354
**Comment:**
*Custom Rule: Add a concrete type annotation for this new ID-indexed
map variable.
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%2F41076&comment_hash=9183f78847fed27264623183130f4276a8294b39ae3bea9e7ccaf7efdb92a576&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=9183f78847fed27264623183130f4276a8294b39ae3bea9e7ccaf7efdb92a576&reaction=dislike'>π</a>
##########
superset/daos/dataset.py:
##########
@@ -363,6 +411,9 @@ def update_metrics(
- If a metric Dict does not have an `id` then we create a new metric.
- If there are extra metrics on the metadata db that are not defined
on the List
then we delete.
+
+ Uses individual ORM operations (not bulk) so that SQLAlchemy-Continuum
+ can capture each row change in the version history.
"""
metrics_by_id = {metric.id: metric for metric in model.metrics}
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly added metric
lookup map. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly added lookup dictionary in modified code with no explicit
type hint, so it violates the type-hint rule for relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=632e957547c1442885dd3172c4693441&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=632e957547c1442885dd3172c4693441&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/daos/dataset.py
**Line:** 419:419
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly added
metric lookup map.
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%2F41076&comment_hash=2a391dcab0b9b7406143a9d68ba13573466f7c6f2d1dd8600d5fef1cc8e17bbd&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=2a391dcab0b9b7406143a9d68ba13573466f7c6f2d1dd8600d5fef1cc8e17bbd&reaction=dislike'>π</a>
##########
superset/daos/version.py:
##########
@@ -0,0 +1,69 @@
+# 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.
+"""Backward-compat façade for the entity-versioning DAO surface.
+
+The actual implementation lives in :mod:`superset.versioning.queries`
+(read side: list/get/resolve/find/UUID derivation). This module
+re-exports it under a single ``VersionDAO`` class plus the module-level
+UUID helpers so existing callers keep working without changes. (The
+write side β restore + audit stamping β ships in a later PR; only the
+read surface is wired here.)
+
+New code should import from the versioning sub-modules directly.
+"""
+
+from __future__ import annotations
+
+from superset.versioning.queries import (
+ current_live_transaction_id,
+ current_live_version_uuid,
+ current_version_number,
+ derive_version_uuid,
+ derive_version_uuid as _derive_version_uuid, # noqa: F401
+ find_active_by_uuid,
+ get_version,
+ list_change_records_batch,
+ list_versions,
+ resolve_version_uuid,
+ VERSION_UUID_NAMESPACE,
+)
+
+# Re-exports for ``from superset.daos.version import β¦`` consumers.
+__all__ = [
+ "VERSION_UUID_NAMESPACE",
+ "VersionDAO",
+ "derive_version_uuid",
+]
Review Comment:
**Suggestion:** Add an explicit type annotation for the exported symbol list
to satisfy the type-hinting requirement for annotatable module-level variables.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The module-level exported symbol list is a clearly annotatable variable, and
the file does not provide a type hint for it. This matches the rule requiring
type hints on relevant variables that can be annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e486b0d74eb54720b9bb79a18272b144&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=e486b0d74eb54720b9bb79a18272b144&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/daos/version.py
**Line:** 46:50
**Comment:**
*Custom Rule: Add an explicit type annotation for the exported symbol
list to satisfy the type-hinting requirement for annotatable module-level
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%2F41076&comment_hash=a6040fea15580f3f8b72c657809aa393a3331e97c80250e7a1dc1e32a77d8c0c&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=a6040fea15580f3f8b72c657809aa393a3331e97c80250e7a1dc1e32a77d8c0c&reaction=dislike'>π</a>
##########
superset/datasets/api.py:
##########
@@ -440,17 +504,69 @@ def put(self, pk: int) -> Response:
# This validates custom Schema with custom validations
except ValidationError as error:
return self.response_400(message=error.messages)
+
+ # Live version identifiers before the update (empty + query-free when
+ # ``ENABLE_VERSIONING_CAPTURE`` is off).
+ old_info = current_entity_version_info(SqlaTable, pk)
Review Comment:
**Suggestion:** Add an explicit type annotation to this newly introduced
variable assignment so the version metadata object is typed. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The new local variable is introduced without an explicit type annotation,
and it is a relevant variable that can be annotated under the Python type-hint
rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=87f987b34e92458195f1b7d6be3b7154&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=87f987b34e92458195f1b7d6be3b7154&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/datasets/api.py
**Line:** 510:510
**Comment:**
*Custom Rule: Add an explicit type annotation to this newly introduced
variable assignment so the version metadata object is 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%2F41076&comment_hash=829d471f5b448dd875154ca0a3910293abab2e3c814e8fda1762852e12072399&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=829d471f5b448dd875154ca0a3910293abab2e3c814e8fda1762852e12072399&reaction=dislike'>π</a>
##########
superset/datasets/api.py:
##########
@@ -440,17 +504,69 @@ def put(self, pk: int) -> Response:
# This validates custom Schema with custom validations
except ValidationError as error:
return self.response_400(message=error.messages)
+
+ # Live version identifiers before the update (empty + query-free when
+ # ``ENABLE_VERSIONING_CAPTURE`` is off).
+ old_info = current_entity_version_info(SqlaTable, pk)
+
try:
+ # Two commands, two commits, two Continuum transactions for an
+ # ``override_columns`` save β deliberately NOT merged into one
+ # transaction. A single-transaction design was attempted and
+ # reverted: ``DBEventLogger`` writes request logs through the
+ # SHARED scoped session and calls ``commit()`` /
+ # ``rollback()`` on it mid-request (superset/utils/log.py),
+ # so any save held uncommitted across a logged sub-action can
+ # be committed half-done (Postgres/MySQL) or rolled back
+ # entirely on a transient logger failure (SQLite's
+ # "database is locked"). Until the event logger gets its own
+ # session, per-command commit boundaries are the only shape
+ # whose failure modes are honest. Consequence the
+ # version-history UI must tolerate: one logical save can
+ # surface as two version transactions stamped the same second.
changed_model = UpdateDatasetCommand(pk, item,
override_columns).run()
+ # Capture the post-update identifiers BEFORE the refresh:
+ # RefreshDatasetCommand commits its own transaction, so reading
+ # afterwards would attribute the refresh's version to the
+ # user's update (and oldβnew would span two transactions).
+ new_info = current_entity_version_info(
+ SqlaTable, changed_model.id, changed_model.uuid
+ )
+ etag_version_uuid = new_info.version_uuid
Review Comment:
**Suggestion:** Annotate this new variable with its concrete optional string
type to comply with the type-hint requirement for relevant variables.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The variable is introduced without a type annotation and is a simple local
value that can be annotated, so this is a real omission under the type-hint
rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ad94bb4d9008401d878241e8aacfcb6d&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=ad94bb4d9008401d878241e8aacfcb6d&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/datasets/api.py
**Line:** 535:535
**Comment:**
*Custom Rule: Annotate this new variable with its concrete optional
string type to comply with the type-hint requirement 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%2F41076&comment_hash=73b727b08894f5c1a9906245c3dfcfeec8bdcd8485625070e53a39168ce6a881&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=73b727b08894f5c1a9906245c3dfcfeec8bdcd8485625070e53a39168ce6a881&reaction=dislike'>π</a>
##########
superset/datasets/api.py:
##########
@@ -440,17 +504,69 @@ def put(self, pk: int) -> Response:
# This validates custom Schema with custom validations
except ValidationError as error:
return self.response_400(message=error.messages)
+
+ # Live version identifiers before the update (empty + query-free when
+ # ``ENABLE_VERSIONING_CAPTURE`` is off).
+ old_info = current_entity_version_info(SqlaTable, pk)
+
try:
+ # Two commands, two commits, two Continuum transactions for an
+ # ``override_columns`` save β deliberately NOT merged into one
+ # transaction. A single-transaction design was attempted and
+ # reverted: ``DBEventLogger`` writes request logs through the
+ # SHARED scoped session and calls ``commit()`` /
+ # ``rollback()`` on it mid-request (superset/utils/log.py),
+ # so any save held uncommitted across a logged sub-action can
+ # be committed half-done (Postgres/MySQL) or rolled back
+ # entirely on a transient logger failure (SQLite's
+ # "database is locked"). Until the event logger gets its own
+ # session, per-command commit boundaries are the only shape
+ # whose failure modes are honest. Consequence the
+ # version-history UI must tolerate: one logical save can
+ # surface as two version transactions stamped the same second.
changed_model = UpdateDatasetCommand(pk, item,
override_columns).run()
+ # Capture the post-update identifiers BEFORE the refresh:
+ # RefreshDatasetCommand commits its own transaction, so reading
+ # afterwards would attribute the refresh's version to the
+ # user's update (and oldβnew would span two transactions).
+ new_info = current_entity_version_info(
+ SqlaTable, changed_model.id, changed_model.uuid
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation for this new local variable
returned from the version helper to satisfy required type hint coverage.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This newly added local variable is also unannotated even though its type is
inferable, so it matches the rule requiring type hints for relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=62f18726ed7d449e8bbfb692915da5ad&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=62f18726ed7d449e8bbfb692915da5ad&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/datasets/api.py
**Line:** 532:534
**Comment:**
*Custom Rule: Add an explicit type annotation for this new local
variable returned from the version helper to satisfy required type hint
coverage.
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%2F41076&comment_hash=fafc9de81b73e957b35bde969ac793e82a14637810636f0ad6abf1e4a0ed82de&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=fafc9de81b73e957b35bde969ac793e82a14637810636f0ad6abf1e4a0ed82de&reaction=dislike'>π</a>
##########
scripts/seed_junction_load.py:
##########
@@ -0,0 +1,679 @@
+#!/usr/bin/env python3
+# 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.
+#
+# ----------------------------------------------------------------------
+# Stress-test data generator for the composite-PK migration (sc-105349).
+#
+# Bulk-inserts synthetic parent rows and many-to-many junction rows for
+# the eight association tables that the composite-PK migration touches.
+# Useful for measuring migration runtime at varying scales β run this at
+# 100K / 1M / 5M / 10M rows and time the migration at each scale to
+# verify the O(N log N) extrapolation.
+#
+# Idempotent: rerunning with the same target is a no-op; rerunning with
+# a higher target adds rows up to the new total. Batched bulk INSERTs
+# (10K rows per statement) make it fast on Postgres, MySQL, and SQLite.
+#
+# Usage (inside the Superset container):
+#
+# docker exec superset-superset-1 \\
+# /app/.venv/bin/python /app/scripts/seed_junction_load.py \\
+# --dashboard-slices 1000000 \\
+# --slice-user 100000 \\
+# --dashboard-user 100000
+#
+# Run with no flags for the defaults shown below. Use ``--dry-run`` to
+# print the planned inserts without writing anything.
+#
+# The script connects via Superset's standard ``DATABASE_*`` env vars
+# (or ``SUPERSET__SQLALCHEMY_DATABASE_URI`` if set), so it works
+# automatically inside the Superset container regardless of which
+# metadata DB backend is in use.
+
+from __future__ import annotations
+
+import argparse
+import logging
+import os
+import sys
+import time
+from contextlib import contextmanager
+from typing import Iterator
+from uuid import uuid4
+
+import sqlalchemy as sa
+from sqlalchemy.engine import Connection, Engine
+
+logger = logging.getLogger("seed_junction_load")
Review Comment:
**Suggestion:** Add an explicit type annotation to this module-level logger
variable (for example, annotate it as a logging logger type) to satisfy the
type-hint requirement for relevant variables. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The module defines a reusable logger without an explicit type annotation,
which matches the custom rule requiring type hints on relevant variables that
can be annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5c77e170ef5145c2856ba48ad38cdd09&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=5c77e170ef5145c2856ba48ad38cdd09&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:** scripts/seed_junction_load.py
**Line:** 62:62
**Comment:**
*Custom Rule: Add an explicit type annotation to this module-level
logger variable (for example, annotate it as a logging logger type) to satisfy
the type-hint requirement 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%2F41076&comment_hash=31d0563a060dc4ed5b4688fe97728c1db4d132102db5695689e3871ae38ceb80&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=31d0563a060dc4ed5b4688fe97728c1db4d132102db5695689e3871ae38ceb80&reaction=dislike'>π</a>
##########
superset/charts/api.py:
##########
@@ -437,9 +496,29 @@ def put(self, pk: int) -> Response:
# This validates custom Schema with custom validations
except ValidationError as error:
return self.response_400(message=error.messages)
+
+ # Live version identifiers before the update (empty + query-free when
+ # ``ENABLE_VERSIONING_CAPTURE`` is off, so this stays inert under the
+ # kill-switch).
+ old_info = current_entity_version_info(Slice, pk)
Review Comment:
**Suggestion:** Add explicit type annotations for the new version-info local
variables to satisfy the type-hint requirement for newly introduced variables.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This newly added local variable is not type-annotated, and it is a value
whose type can reasonably be annotated under the Python type-hint rule for
relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ff9dd4f2faa74127bd3c7f1e787feba6&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=ff9dd4f2faa74127bd3c7f1e787feba6&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/api.py
**Line:** 503:503
**Comment:**
*Custom Rule: Add explicit type annotations for the new version-info
local variables 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%2F41076&comment_hash=37b39fba1a3763e0b200e1ba3ecdb300a9be6e8cba956fab4ed5680638a495c7&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=37b39fba1a3763e0b200e1ba3ecdb300a9be6e8cba956fab4ed5680638a495c7&reaction=dislike'>π</a>
##########
scripts/seed_junction_load.py:
##########
@@ -0,0 +1,679 @@
+#!/usr/bin/env python3
+# 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.
+#
+# ----------------------------------------------------------------------
+# Stress-test data generator for the composite-PK migration (sc-105349).
+#
+# Bulk-inserts synthetic parent rows and many-to-many junction rows for
+# the eight association tables that the composite-PK migration touches.
+# Useful for measuring migration runtime at varying scales β run this at
+# 100K / 1M / 5M / 10M rows and time the migration at each scale to
+# verify the O(N log N) extrapolation.
+#
+# Idempotent: rerunning with the same target is a no-op; rerunning with
+# a higher target adds rows up to the new total. Batched bulk INSERTs
+# (10K rows per statement) make it fast on Postgres, MySQL, and SQLite.
+#
+# Usage (inside the Superset container):
+#
+# docker exec superset-superset-1 \\
+# /app/.venv/bin/python /app/scripts/seed_junction_load.py \\
+# --dashboard-slices 1000000 \\
+# --slice-user 100000 \\
+# --dashboard-user 100000
+#
+# Run with no flags for the defaults shown below. Use ``--dry-run`` to
+# print the planned inserts without writing anything.
+#
+# The script connects via Superset's standard ``DATABASE_*`` env vars
+# (or ``SUPERSET__SQLALCHEMY_DATABASE_URI`` if set), so it works
+# automatically inside the Superset container regardless of which
+# metadata DB backend is in use.
+
+from __future__ import annotations
+
+import argparse
+import logging
+import os
+import sys
+import time
+from contextlib import contextmanager
+from typing import Iterator
+from uuid import uuid4
+
+import sqlalchemy as sa
+from sqlalchemy.engine import Connection, Engine
+
+logger = logging.getLogger("seed_junction_load")
+
+# Bulk INSERT batch size. Larger values = fewer statements but more memory.
+BATCH = 10_000
Review Comment:
**Suggestion:** Annotate this module-level constant with an explicit integer
type so the variable is fully type-hinted. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This module-level integer constant is unannotated even though it can easily
be typed, so it fits the Python type-hint rule for relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c51526c5a0164da6bbbdcfa00c68778b&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=c51526c5a0164da6bbbdcfa00c68778b&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:** scripts/seed_junction_load.py
**Line:** 65:65
**Comment:**
*Custom Rule: Annotate this module-level constant with an explicit
integer type so the variable is fully type-hinted.
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%2F41076&comment_hash=faa76e5edb16513ebaa7750956d7e861fb3935e1db206e4bd31bff561bf96524&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=faa76e5edb16513ebaa7750956d7e861fb3935e1db206e4bd31bff561bf96524&reaction=dislike'>π</a>
##########
scripts/seed_junction_load.py:
##########
@@ -0,0 +1,679 @@
+#!/usr/bin/env python3
+# 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.
+#
+# ----------------------------------------------------------------------
+# Stress-test data generator for the composite-PK migration (sc-105349).
+#
+# Bulk-inserts synthetic parent rows and many-to-many junction rows for
+# the eight association tables that the composite-PK migration touches.
+# Useful for measuring migration runtime at varying scales β run this at
+# 100K / 1M / 5M / 10M rows and time the migration at each scale to
+# verify the O(N log N) extrapolation.
+#
+# Idempotent: rerunning with the same target is a no-op; rerunning with
+# a higher target adds rows up to the new total. Batched bulk INSERTs
+# (10K rows per statement) make it fast on Postgres, MySQL, and SQLite.
+#
+# Usage (inside the Superset container):
+#
+# docker exec superset-superset-1 \\
+# /app/.venv/bin/python /app/scripts/seed_junction_load.py \\
+# --dashboard-slices 1000000 \\
+# --slice-user 100000 \\
+# --dashboard-user 100000
+#
+# Run with no flags for the defaults shown below. Use ``--dry-run`` to
+# print the planned inserts without writing anything.
+#
+# The script connects via Superset's standard ``DATABASE_*`` env vars
+# (or ``SUPERSET__SQLALCHEMY_DATABASE_URI`` if set), so it works
+# automatically inside the Superset container regardless of which
+# metadata DB backend is in use.
+
+from __future__ import annotations
+
+import argparse
+import logging
+import os
+import sys
+import time
+from contextlib import contextmanager
+from typing import Iterator
+from uuid import uuid4
+
+import sqlalchemy as sa
+from sqlalchemy.engine import Connection, Engine
+
+logger = logging.getLogger("seed_junction_load")
+
+# Bulk INSERT batch size. Larger values = fewer statements but more memory.
+BATCH = 10_000
+
+# Default per-junction-table target row counts. Tuned to mimic the shape
+# of a large multi-team Superset install. Override via CLI flags.
+DEFAULTS: dict[str, int] = {
+ "dashboard_slices": 1_000_000,
+ "slice_user": 100_000,
+ "dashboard_user": 100_000,
+ "dashboard_roles": 10_000,
+}
+
+# (junction_table, fk1_col, fk2_col, parent1_table, parent2_table)
+# parents reference id columns; we generate (fk1, fk2) pairs by sampling
+# from the parents' existing IDs.
+JUNCTIONS: list[tuple[str, str, str, str, str]] = [
+ ("dashboard_slices", "dashboard_id", "slice_id", "dashboards", "slices"),
+ ("slice_user", "user_id", "slice_id", "ab_user", "slices"),
+ ("dashboard_user", "user_id", "dashboard_id", "ab_user", "dashboards"),
+ ("dashboard_roles", "dashboard_id", "role_id", "dashboards", "ab_role"),
+]
+
+# Junction tables that originally carried ``UNIQUE(fk1, fk2)`` and therefore
+# cannot accept duplicate ``(fk1, fk2)`` pairs even on the pre-migration
+# (downgrade) schema. The other JUNCTIONS allow duplicates pre-migration.
+# Only ``dashboard_slices`` is listed: the migration's other UNIQUE table
+# (``report_schedule_user``) is not in JUNCTIONS β this script doesn't seed
+# it β so listing it here would imply coverage that doesn't exist. Add it
+# alongside a JUNCTIONS entry if that table ever gets seeded.
+JUNCTIONS_WITH_UNIQUE: set[str] = {"dashboard_slices"}
+
+
+# ----------------------------------------------------------------------
+# Connection setup
+# ----------------------------------------------------------------------
+
+
+def build_engine() -> Engine:
+ """Build a SQLAlchemy engine from Superset env vars."""
+ if uri := os.environ.get("SUPERSET__SQLALCHEMY_DATABASE_URI"):
+ logger.info("Using SUPERSET__SQLALCHEMY_DATABASE_URI from env")
+ return sa.create_engine(uri)
+
+ try:
+ dialect = os.environ["DATABASE_DIALECT"]
+ user = os.environ["DATABASE_USER"]
+ password = os.environ["DATABASE_PASSWORD"]
+ host = os.environ["DATABASE_HOST"]
+ port = os.environ["DATABASE_PORT"]
+ db = os.environ["DATABASE_DB"]
+ except KeyError as exc:
+ sys.exit(
+ f"Missing env var {exc}; either set
DATABASE_DIALECT/USER/PASSWORD/"
+ f"HOST/PORT/DB or SUPERSET__SQLALCHEMY_DATABASE_URI before
running."
+ )
+
+ uri = f"{dialect}://{user}:{password}@{host}:{port}/{db}"
+ logger.info(
+ "Built URI from DATABASE_* env vars (dialect=%s, host=%s)", dialect,
host
+ )
+ return sa.create_engine(uri)
+
+
+# ----------------------------------------------------------------------
+# Helpers
+# ----------------------------------------------------------------------
+
+
+def uuid_value(dialect_name: str) -> bytes | str:
+ """Return a UUID in the form the active dialect expects.
+
+ MySQL stores UUIDs as ``BINARY(16)`` (16 raw bytes); Postgres has a
+ native ``UUID`` type that accepts strings; SQLite stores them as
+ BLOB/TEXT and accepts either. Branching here keeps the seed script
+ backend-agnostic without depending on Superset's custom column types.
+ """
+ if dialect_name.startswith("mysql"):
+ return uuid4().bytes
+ return str(uuid4())
+
+
+@contextmanager
+def time_phase(name: str) -> Iterator[None]:
+ """Log elapsed wall time for a named phase."""
+ start = time.monotonic()
+ logger.info("[%s] starting", name)
+ try:
+ yield
+ finally:
+ elapsed = time.monotonic() - start
+ logger.info("[%s] done in %.2fs", name, elapsed)
+
+
+def count_rows(conn: Connection, table: str) -> int:
+ return conn.scalar(sa.text(f"SELECT COUNT(*) FROM {table}")) or 0 # noqa:
S608
+
+
+def existing_ids(conn: Connection, table: str, limit: int | None = None) ->
list[int]:
+ sql = f"SELECT id FROM {table} ORDER BY id" # noqa: S608
+ if limit is not None:
+ sql += f" LIMIT {limit}"
+ return [row[0] for row in conn.execute(sa.text(sql))]
+
+
+# ----------------------------------------------------------------------
+# Parent seeders
+#
+# Each function ensures the named parent table has at least ``target``
+# rows by inserting synthetic ones with minimal-but-valid columns.
+# Returns nothing; subsequent code reads back IDs via ``existing_ids``.
+# ----------------------------------------------------------------------
+
+
+def seed_dashboards(conn: Connection, target: int, dry_run: bool) -> None:
+ current = count_rows(conn, "dashboards")
+ if current >= target:
+ logger.info(
+ "dashboards: %d rows (target %d) β no insert needed", current,
target
+ )
+ return
+ needed = target - current
+ logger.info("dashboards: %d β %d (+%d)", current, target, needed)
+ if dry_run:
+ return
+
+ dialect = conn.engine.dialect.name
+ sql = sa.text(
+ "INSERT INTO dashboards (uuid, dashboard_title, slug, published) "
+ "VALUES (:uuid, :title, :slug, :published)"
+ )
+ for batch_start in range(0, needed, BATCH):
+ rows = [
+ {
+ "uuid": uuid_value(dialect),
+ "title": f"seed_dashboard_{current + i}",
+ "slug": f"seed-dashboard-{current + i}-{uuid4().hex[:8]}",
+ "published": False,
+ }
+ for i in range(batch_start, min(batch_start + BATCH, needed))
+ ]
+ conn.execute(sql, rows)
+ logger.info(" dashboards: inserted %d / %d", batch_start + len(rows),
needed)
+
+
+def seed_dbs(conn: Connection, dry_run: bool) -> int:
+ """Ensure at least one row exists in ``dbs`` (parent of ``tables``).
+ Returns the id to use as ``database_id`` when seeding ``tables``."""
+ ids = existing_ids(conn, "dbs", limit=1)
+ if ids:
+ return ids[0]
+ if dry_run:
+ return -1 # placeholder
+ dialect = conn.engine.dialect.name
+ logger.info("dbs: inserting one synthetic database (no rows present)")
+ conn.execute(
+ sa.text(
+ "INSERT INTO dbs (uuid, database_name, sqlalchemy_uri,
expose_in_sqllab) "
+ "VALUES (:uuid, :name, :uri, :expose)"
+ ),
+ {
+ "uuid": uuid_value(dialect),
+ "name": f"seed_db_{uuid4().hex[:8]}",
+ "uri": "sqlite:///seed.db",
+ "expose": False,
+ },
+ )
+ return existing_ids(conn, "dbs", limit=1)[0]
+
+
+def seed_tables(conn: Connection, target: int, dry_run: bool) -> None:
+ current = count_rows(conn, "tables")
+ if current >= target:
+ logger.info("tables: %d rows (target %d) β no insert needed", current,
target)
+ return
+ needed = target - current
+ logger.info("tables: %d β %d (+%d)", current, target, needed)
+ if dry_run:
+ return
+
+ database_id = seed_dbs(conn, dry_run=False)
+ dialect = conn.engine.dialect.name
+ sql = sa.text(
+ "INSERT INTO tables (uuid, table_name, database_id) "
+ "VALUES (:uuid, :name, :db_id)"
+ )
+ for batch_start in range(0, needed, BATCH):
+ rows = [
+ {
+ "uuid": uuid_value(dialect),
+ "name": f"seed_table_{current + i}",
+ "db_id": database_id,
+ }
+ for i in range(batch_start, min(batch_start + BATCH, needed))
+ ]
+ conn.execute(sql, rows)
+ logger.info(" tables: inserted %d / %d", batch_start + len(rows),
needed)
+
+
+def seed_slices(conn: Connection, target: int, dry_run: bool) -> None:
+ current = count_rows(conn, "slices")
+ if current >= target:
+ logger.info("slices: %d rows (target %d) β no insert needed", current,
target)
+ return
+ needed = target - current
+ logger.info("slices: %d β %d (+%d)", current, target, needed)
+ if dry_run:
+ return
+
+ # Slices reference tables.id; ensure at least one ``tables`` row exists
+ # so the FK is satisfiable (datasource_id is nullable but we set it for
+ # realism). The migration test doesn't care, but a real Superset that
+ # re-renders these slices does.
+ seed_tables(conn, target=1, dry_run=False)
+ table_id = existing_ids(conn, "tables", limit=1)[0]
+ dialect = conn.engine.dialect.name
+ sql = sa.text(
+ "INSERT INTO slices "
+ "(uuid, slice_name, datasource_id, datasource_type, viz_type) "
+ "VALUES (:uuid, :name, :ds_id, :ds_type, :viz)"
+ )
+ for batch_start in range(0, needed, BATCH):
+ rows = [
+ {
+ "uuid": uuid_value(dialect),
+ "name": f"seed_slice_{current + i}",
+ "ds_id": table_id,
+ "ds_type": "table",
+ "viz": "table",
+ }
+ for i in range(batch_start, min(batch_start + BATCH, needed))
+ ]
+ conn.execute(sql, rows)
+ logger.info(" slices: inserted %d / %d", batch_start + len(rows),
needed)
+
+
+def seed_users(conn: Connection, target: int, dry_run: bool) -> None:
+ current = count_rows(conn, "ab_user")
+ if current >= target:
+ logger.info("ab_user: %d rows (target %d) β no insert needed",
current, target)
+ return
+ needed = target - current
+ logger.info("ab_user: %d β %d (+%d)", current, target, needed)
+ if dry_run:
+ return
+
+ sql = sa.text(
+ "INSERT INTO ab_user (first_name, last_name, username, email, active) "
+ "VALUES (:first, :last, :username, :email, :active)"
+ )
+ for batch_start in range(0, needed, BATCH):
+ rows = [
+ {
+ "first": "seed",
+ "last": f"user_{current + i}",
+ "username": f"seed_user_{current + i}_{uuid4().hex[:8]}",
+ "email": f"seed_user_{current +
i}_{uuid4().hex[:8]}@example.invalid",
+ "active": True,
+ }
+ for i in range(batch_start, min(batch_start + BATCH, needed))
+ ]
+ conn.execute(sql, rows)
+ logger.info(" ab_user: inserted %d / %d", batch_start + len(rows),
needed)
+
+
+def seed_roles(conn: Connection, target: int, dry_run: bool) -> None:
+ current = count_rows(conn, "ab_role")
+ if current >= target:
+ logger.info("ab_role: %d rows (target %d) β no insert needed",
current, target)
+ return
+ needed = target - current
+ logger.info("ab_role: %d β %d (+%d)", current, target, needed)
+ if dry_run:
+ return
+
+ sql = sa.text("INSERT INTO ab_role (name) VALUES (:name)")
+ for batch_start in range(0, needed, BATCH):
+ rows = [
+ {"name": f"seed_role_{current + i}_{uuid4().hex[:8]}"}
+ for i in range(batch_start, min(batch_start + BATCH, needed))
+ ]
+ conn.execute(sql, rows)
+ logger.info(" ab_role: inserted %d / %d", batch_start + len(rows),
needed)
+
+
+# ----------------------------------------------------------------------
+# Junction seeder
+# ----------------------------------------------------------------------
+
+
+def _load_existing_pairs(
+ conn: Connection, junction: str, fk1_col: str, fk2_col: str
+) -> set[tuple[int, int]]:
+ """Load existing ``(fk1, fk2)`` pairs from a junction table into a set.
+
+ Used so the seeder can skip them when generating new pairs (junction
+ tables enforce uniqueness on the FK pair). Memory is ~32 bytes/tuple
+ on CPython, so 10M existing pairs is ~320MB β acceptable for a dev
+ machine. The junction / column names come from ``JUNCTIONS``, not
+ user input, so the f-string interpolation is safe.
+ """
+ sql_text = f"SELECT {fk1_col}, {fk2_col} FROM {junction}" # noqa: S608
+ return {(row[0], row[1]) for row in conn.execute(sa.text(sql_text))}
+
+
+def _generate_new_pairs(
+ p1_ids: list[int],
+ p2_ids: list[int],
+ existing_pairs: set[tuple[int, int]],
+) -> Iterator[tuple[int, int]]:
+ """Yield ``(fk1, fk2)`` pairs from the parent1 Γ parent2 cross-product
+ that are not already in ``existing_pairs``."""
+ for fk1 in p1_ids:
+ for fk2 in p2_ids:
+ if (fk1, fk2) not in existing_pairs:
+ yield (fk1, fk2)
+
+
+def seed_junction(
+ conn: Connection,
+ junction: str,
+ fk1_col: str,
+ fk2_col: str,
+ parent1: str,
+ parent2: str,
+ target: int,
+ dry_run: bool,
+) -> None:
+ """Bulk-insert junction rows up to ``target`` rows total.
+
+ Generates ``(fk1, fk2)`` pairs by walking the cross-product of
+ parent1 IDs Γ parent2 IDs in row-major order, skipping pairs that
+ already exist. Walking the cross-product deterministically keeps
+ the script replayable: re-running with the same target is a no-op,
+ and re-running with a higher target appends new pairs in a stable
+ order regardless of how many runs preceded.
+ """
+ current = count_rows(conn, junction)
+ if current >= target:
+ logger.info(
+ "%s: %d rows (target %d) β no insert needed", junction, current,
target
+ )
+ return
+ needed = target - current
+ logger.info("%s: %d β %d (+%d)", junction, current, target, needed)
+ if dry_run:
+ return
+
+ p1_ids = existing_ids(conn, parent1)
+ p2_ids = existing_ids(conn, parent2)
+ max_pairs = len(p1_ids) * len(p2_ids)
+ if max_pairs < target:
+ sys.exit(
+ f"Cannot reach {target} rows in {junction}: "
+ f"only {max_pairs} unique pairs available "
+ f"({len(p1_ids)} Γ {len(p2_ids)}). "
+ f"Increase parent targets and rerun."
+ )
+
+ existing_pairs: set[tuple[int, int]] = (
+ _load_existing_pairs(conn, junction, fk1_col, fk2_col) if current > 0
else set()
+ )
+ if existing_pairs:
+ logger.info(
+ " %s: loaded %d existing pairs into avoidance set",
+ junction,
+ len(existing_pairs),
+ )
+
+ insert_sql = sa.text(
+ f"INSERT INTO {junction} ({fk1_col}, {fk2_col}) " # noqa: S608
+ f"VALUES (:fk1, :fk2)"
+ )
+
+ inserted = 0
+ batch: list[dict[str, int]] = []
+ for fk1, fk2 in _generate_new_pairs(p1_ids, p2_ids, existing_pairs):
+ batch.append({"fk1": fk1, "fk2": fk2})
+ inserted += 1
+ if len(batch) == BATCH or inserted == needed:
+ conn.execute(insert_sql, batch)
+ logger.info(" %s: inserted %d / %d", junction, inserted, needed)
+ batch = []
+ if inserted == needed:
+ return
+ if inserted < needed:
+ sys.exit(
+ f"Ran out of unique pairs at {inserted}/{needed} for {junction} "
+ f"(parents have {len(p1_ids)} Γ {len(p2_ids)} = {max_pairs} pairs,
"
+ f"{len(existing_pairs)} already present)"
+ )
+
+
+# ----------------------------------------------------------------------
+# Orchestration
+# ----------------------------------------------------------------------
+
+
+def _compute_parent_requirements(targets: dict[str, int]) -> dict[str, int]:
+ """For each parent table, return the minimum row count needed so that
+ parent1 Γ parent2 β₯ target for every junction it participates in.
+
+ Allocates ceil(sqrt(target)) rows per parent, balanced across the two
+ parents of each junction. The actual junction seeder will then walk
+ the cross-product to produce the target number of unique pairs.
+ """
+ parent_req: dict[str, int] = {}
+ for junction, _, _, p1, p2 in JUNCTIONS:
+ target = targets.get(junction, 0)
+ if target == 0:
+ continue
+ sqrt_n = int(target**0.5) + 1
+ parent_req[p1] = max(parent_req.get(p1, 0), sqrt_n)
+ parent_req[p2] = max(parent_req.get(p2, 0), sqrt_n)
+ return parent_req
+
+
+def _seed_parents(conn: Connection, parent_req: dict[str, int], dry_run: bool)
-> None:
+ """Seed parent tables in dependency order:
+ independent parents (ab_user, ab_role) first, then dashboards / slices /
+ tables (which transitively depend on dbs, seeded inside seed_tables)."""
+ if "ab_user" in parent_req:
+ seed_users(conn, parent_req["ab_user"], dry_run)
+ if "ab_role" in parent_req:
+ seed_roles(conn, parent_req["ab_role"], dry_run)
+ if "dashboards" in parent_req:
+ seed_dashboards(conn, parent_req["dashboards"], dry_run)
+ if "slices" in parent_req:
+ seed_slices(conn, parent_req["slices"], dry_run)
+ if "tables" in parent_req:
+ seed_tables(conn, parent_req["tables"], dry_run)
+
+
+def _seed_all_junctions(
+ conn: Connection, targets: dict[str, int], dry_run: bool
+) -> None:
+ for junction, fk1, fk2, p1, p2 in JUNCTIONS:
+ target = targets.get(junction, 0)
+ if target == 0:
+ continue
+ with time_phase(f"junction:{junction}"):
+ seed_junction(conn, junction, fk1, fk2, p1, p2, target, dry_run)
+
+
+def inject_duplicates(
+ conn: Connection,
+ junction: str,
+ fk1_col: str,
+ fk2_col: str,
+ pct: float,
+ dry_run: bool,
+) -> None:
+ """Insert duplicate ``(fk1, fk2)`` rows on a non-UNIQUE junction table.
+
+ Used to stress-test the migration's ``_dedupe_by_min_id`` phase, which
+ is otherwise a no-op on cleanly-seeded data. Computes ``count =
+ current_rows * pct / 100`` and inserts that many rows by re-sampling
+ existing ``(fk1, fk2)`` pairs in row-major order. The synthetic
+ duplicates land on top of distinct existing pairs (one duplicate per
+ distinct pair, then wraps), so the migration's dedupe finds and
+ deletes them.
+
+ **Pre-condition: the table must NOT have UNIQUE on (fk1, fk2)**, i.e.,
+ the schema must be the pre-migration shape (after running
+ ``superset db downgrade``). On the post-migration schema the composite
+ PK rejects duplicates and this function will error.
+ """
+ if pct == 0:
+ return
+ current = count_rows(conn, junction)
+ count = int(current * pct / 100)
+ if count == 0:
+ logger.info(
+ "%s: 0 duplicates to inject (current=%d, pct=%g)",
+ junction,
+ current,
+ pct,
+ )
+ return
+ logger.info(
+ "%s: injecting %d duplicate rows (%g%% of %d existing)",
+ junction,
+ count,
+ pct,
+ current,
+ )
+ if dry_run:
+ return
+
+ select_sql = sa.text(
+ f"SELECT {fk1_col}, {fk2_col} FROM {junction} ORDER BY id LIMIT :n" #
noqa: S608
+ )
+ sample = conn.execute(select_sql, {"n": count}).fetchall()
+ if not sample:
+ logger.warning("%s: no rows to duplicate (table is empty)", junction)
+ return
+
+ insert_sql = sa.text(
+ f"INSERT INTO {junction} ({fk1_col}, {fk2_col}) " # noqa: S608
+ f"VALUES (:fk1, :fk2)"
+ )
+ inserted = 0
+ while inserted < count:
+ batch: list[dict[str, int]] = []
+ while len(batch) < BATCH and inserted < count:
+ row = sample[inserted % len(sample)]
+ batch.append({"fk1": row[0], "fk2": row[1]})
+ inserted += 1
+ conn.execute(insert_sql, batch)
+ logger.info(" %s: injected %d / %d duplicates", junction, inserted,
count)
+
+
+def _inject_dirty_data(conn: Connection, dirty_pct: float, dry_run: bool) ->
None:
+ """Inject duplicate rows on every non-UNIQUE seeded junction.
+
+ The two tables that originally carried ``UNIQUE(fk1, fk2)`` are
+ skipped because their composite-PK successor (and their pre-migration
+ UNIQUE constraint) both reject duplicate inserts.
+ """
+ if dirty_pct == 0:
+ return
+ for junction, fk1, fk2, _, _ in JUNCTIONS:
+ if junction in JUNCTIONS_WITH_UNIQUE:
+ logger.info(
+ "%s: skipping duplicate injection (table has UNIQUE on FK
pair)",
+ junction,
+ )
+ continue
+ with time_phase(f"dirty:{junction}"):
+ inject_duplicates(conn, junction, fk1, fk2, dirty_pct, dry_run)
+
+
+def run(targets: dict[str, int], dry_run: bool, dirty_duplicates_pct: float)
-> None:
+ engine = build_engine()
+ with engine.begin() as conn:
+ parent_req = _compute_parent_requirements(targets)
+ logger.info("Required parent row counts: %s", parent_req)
+
+ with time_phase("parents"):
+ _seed_parents(conn, parent_req, dry_run)
+
+ with time_phase("junctions"):
+ _seed_all_junctions(conn, targets, dry_run)
+
+ if dirty_duplicates_pct > 0:
+ with time_phase("dirty-duplicates"):
+ _inject_dirty_data(conn, dirty_duplicates_pct, dry_run)
+
+
+# ----------------------------------------------------------------------
+# CLI
+# ----------------------------------------------------------------------
+
+
+def main() -> None:
+ parser = argparse.ArgumentParser(
+ description=__doc__,
+ formatter_class=argparse.RawDescriptionHelpFormatter,
+ )
+ for table, default in DEFAULTS.items():
+ parser.add_argument(
+ f"--{table.replace('_', '-')}",
+ type=int,
+ default=default,
+ help=f"target row count for {table} (default: {default:,})",
+ )
+ parser.add_argument(
+ "--dry-run",
+ "-n",
+ action="store_true",
+ help="print planned inserts without writing to the DB",
+ )
+ parser.add_argument(
+ "--dirty-duplicates-pct",
+ type=float,
+ default=0,
+ help=(
+ "after seeding distinct pairs, inject this percentage of duplicate
"
+ "rows on each non-UNIQUE junction (slice_user, dashboard_user, "
+ "dashboard_roles). Stress-tests the migration's _dedupe_by_min_id "
+ "phase. Requires 2bee73611e32 to NOT be applied: un-apply it by "
+ "downgrading to its parent (`superset db downgrade
<down_revision>`, "
+ "where <down_revision> is read from the 2bee73611e32 migration "
+ "file) β the post-migration composite PK rejects duplicates and "
+ "this will error. Default: 0 (no duplicates)."
+ ),
+ )
+ parser.add_argument(
+ "--verbose",
+ "-v",
+ action="store_true",
+ help="increase log verbosity",
+ )
+ args = parser.parse_args()
+
+ logging.basicConfig(
+ level=logging.DEBUG if args.verbose else logging.INFO,
+ format="%(asctime)s [%(levelname)s] %(message)s",
+ datefmt="%H:%M:%S",
+ )
+
+ targets = {table: getattr(args, table) for table in DEFAULTS}
Review Comment:
**Suggestion:** Add an explicit type annotation for this computed mapping so
the variableβs key/value types are declared. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The local variable `targets` is a derived mapping with an obvious `dict[str,
int]` shape, but it is assigned without an explicit annotation, which is
exactly the kind of omission the rule flags.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6110dee3d3fe4a2cbc061e5f6cd7fa2a&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=6110dee3d3fe4a2cbc061e5f6cd7fa2a&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:** scripts/seed_junction_load.py
**Line:** 664:664
**Comment:**
*Custom Rule: Add an explicit type annotation for this computed mapping
so the variableβs key/value types are declared.
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%2F41076&comment_hash=b6c40ca1b921657754596c67862b5ceef35ad248bdbbda3e24f62e2639c8e587&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=b6c40ca1b921657754596c67862b5ceef35ad248bdbbda3e24f62e2639c8e587&reaction=dislike'>π</a>
##########
superset/commands/dashboard/update.py:
##########
@@ -59,23 +59,31 @@ def __init__(self, model_id: int, data: dict[str, Any]):
def run(self) -> Model:
self.validate()
assert self._model is not None
- self.process_tab_diff()
- self.process_native_filter_diff()
-
- # Update tags
- if (tags := self._properties.pop("tags", None)) is not None:
- update_tags(ObjectType.dashboard, self._model.id,
self._model.tags, tags)
-
- # Re-serialize position_json to escape 4-byte Unicode characters
- if position_json := self._properties.get("position_json"):
- self._properties["position_json"] =
json.dumps(json.loads(position_json))
-
- dashboard = DashboardDAO.update(self._model, self._properties)
- if self._properties.get("json_metadata"):
- DashboardDAO.set_dash_metadata(
- dashboard,
- data=json.loads(self._properties.get("json_metadata", "{}")),
- )
+ # Suppress autoflush during the update body so that Continuum's
+ # before_flush baseline listener does not fire mid-operation while
+ # the session is only partially populated.
+ with db.session.no_autoflush:
+ self.process_tab_diff()
+ self.process_native_filter_diff()
+
+ # Update tags
+ if (tags := self._properties.pop("tags", None)) is not None:
+ update_tags(
+ ObjectType.dashboard, self._model.id, self._model.tags,
tags
+ )
+
+ # Re-serialize position_json to escape 4-byte Unicode characters
+ if position_json := self._properties.get("position_json"):
+ self._properties["position_json"] = json.dumps(
+ json.loads(position_json)
+ )
+
+ dashboard = DashboardDAO.update(self._model, self._properties)
Review Comment:
**Suggestion:** Add an explicit type annotation for the new local variable
so the modified method complies with the type-hint requirement. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The modified method introduces a new local variable without a type hint, and
this variable can reasonably be annotated. That matches the stated Python
type-hint rule for relevant variables in modified code.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1a6db024d7344836bc4bd1631c04fe2f&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=1a6db024d7344836bc4bd1631c04fe2f&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/update.py
**Line:** 81:81
**Comment:**
*Custom Rule: Add an explicit type annotation for the new local
variable so the modified method complies with 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%2F41076&comment_hash=21b38d8f89c75784df8f8a99cb9c9acb0509727c88deb8845d1c1d5092443a87&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=21b38d8f89c75784df8f8a99cb9c9acb0509727c88deb8845d1c1d5092443a87&reaction=dislike'>π</a>
##########
superset/charts/api.py:
##########
@@ -437,9 +496,29 @@ def put(self, pk: int) -> Response:
# This validates custom Schema with custom validations
except ValidationError as error:
return self.response_400(message=error.messages)
+
+ # Live version identifiers before the update (empty + query-free when
+ # ``ENABLE_VERSIONING_CAPTURE`` is off, so this stays inert under the
+ # kill-switch).
+ old_info = current_entity_version_info(Slice, pk)
+
try:
changed_model = UpdateChartCommand(pk, item).run()
- response = self.response(200, id=changed_model.id, result=item)
+ new_info = current_entity_version_info(
+ Slice, changed_model.id, changed_model.uuid
+ )
+ response = self.response(
Review Comment:
**Suggestion:** Add an explicit type annotation to the new response local
variable created in this updated block. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The updated code introduces a new local variable without a type hint. Since
the variable is a concrete response object and can be annotated, this matches
the type-hint requirement.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=574d707445d740438abc4a3fb4ca6f51&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=574d707445d740438abc4a3fb4ca6f51&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/api.py
**Line:** 510:510
**Comment:**
*Custom Rule: Add an explicit type annotation to the new response local
variable created in this updated block.
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%2F41076&comment_hash=6d8d71598a4319f3ce9ac5d2617a19c104d453756e0040560de6757aeebcbae8&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=6d8d71598a4319f3ce9ac5d2617a19c104d453756e0040560de6757aeebcbae8&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]
