codeant-ai-for-open-source[bot] commented on code in PR #41075:
URL: https://github.com/apache/superset/pull/41075#discussion_r3500132682
##########
superset/datasets/api.py:
##########
@@ -440,17 +503,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:** Add an explicit type annotation for `etag_version_uuid`
(nullable string) since this new variable has a stable, inferable type.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly introduced local variable in the modified code and its type
is clear from context. Since the rule flags omitted type hints for relevant
variables that can be annotated, this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2e1a730314bd417d8c5aade9cd338cef&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=2e1a730314bd417d8c5aade9cd338cef&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:** 534:534
**Comment:**
*Custom Rule: Add an explicit type annotation for `etag_version_uuid`
(nullable string) since this new variable has a stable, inferable type.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=2bf43517e7967d5974257790bfc748982ec65526a4f66c7b3e0201a2c028ad62&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=2bf43517e7967d5974257790bfc748982ec65526a4f66c7b3e0201a2c028ad62&reaction=dislike'>π</a>
##########
superset/datasets/api.py:
##########
@@ -440,17 +503,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
if override_columns:
RefreshDatasetCommand(pk).run()
- response = self.response(200, id=changed_model.id, result=item)
+ # The ETag must reflect the entity's *current live* version,
+ # which after the refresh is the refresh's transaction β
+ # re-read it rather than reusing the pre-refresh uuid.
+ etag_version_uuid = current_entity_etag_uuid(
+ SqlaTable, changed_model.id, changed_model.uuid
+ )
+ response = self.response(
Review Comment:
**Suggestion:** Add a `Response` type annotation for the newly introduced
`response` local to comply with the rule requiring type hints in modified code.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The code introduces a new local variable without a type annotation in
modified Python code. The custom rule explicitly requires type hints for
variables that can be annotated, so this is a valid violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3fbb53203f424a19a7a7f014ef029cf2&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=3fbb53203f424a19a7a7f014ef029cf2&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:** 543:543
**Comment:**
*Custom Rule: Add a `Response` type annotation for the newly introduced
`response` local to comply with the rule requiring type hints in modified code.
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%2F41075&comment_hash=f9bcbc833e03e56bf947afcbf2acd96a5d17a7084aa48bc5700c98611c93f560&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=f9bcbc833e03e56bf947afcbf2acd96a5d17a7084aa48bc5700c98611c93f560&reaction=dislike'>π</a>
##########
superset/datasets/api.py:
##########
@@ -440,17 +503,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 variable type annotation for `old_info` to
satisfy the type-hint requirement for newly introduced variables. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly introduced local variable in modified Python code, and it
has an inferable type from the helper return value. The custom rule requires
type hints for relevant variables that can be annotated, so the omission is a
real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=189b080563e146ceba0d3b55784fbea1&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=189b080563e146ceba0d3b55784fbea1&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:** 509:509
**Comment:**
*Custom Rule: Add an explicit variable type annotation for `old_info`
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%2F41075&comment_hash=3eb620818de9a2b3b00d2eb60791431dfc3ee8ee6357cf9061d5514d73acbf64&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=3eb620818de9a2b3b00d2eb60791431dfc3ee8ee6357cf9061d5514d73acbf64&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]
