codeant-ai-for-open-source[bot] commented on code in PR #39469:
URL: https://github.com/apache/superset/pull/39469#discussion_r3602381830
##########
superset/migrations/versions/2026-06-23_03-23_a7d3f1b9c2e4_cleanup_stale_can_import_pvm.py:
##########
@@ -112,9 +112,9 @@ def downgrade() -> None:
session = Session(bind=bind)
do_downgrade(session)
try:
- session.commit()
+ session.commit() # pylint: disable=consider-using-transaction
Review Comment:
**Suggestion:** The same explicit `session.commit()` in `downgrade()` has
the same transaction-boundary problem and can desynchronize downgrade behavior
from Alembic’s transaction handling; this should also remain under
Alembic-managed transaction control with flush-only ORM persistence. [logic
error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Downgrade transaction boundaries inconsistent with Alembic env.py
control.
- ⚠️ Rollback of failed downgrade cannot fully revert permission changes.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run database downgrades via Superset (for example `superset db
downgrade`), which uses
Alembic’s env runner at `superset/migrations/env.py:87-142` to call
`run_migrations_online()` and, for MySQL/SQLite, sets
`transaction_per_migration=True` and
`transactional_ddl=True` (line 118-119) while wrapping
`context.run_migrations()` inside
`context.begin_transaction()` (lines 131-133).
2. During this migration run, Alembic executes revision `a7d3f1b9c2e4`’s
`downgrade()`
function defined at
`superset/migrations/versions/2026-06-23_03-23_a7d3f1b9c2e4_cleanup_stale_can_import_pvm.py:110-120`,
which creates `session = Session(bind=bind)` (line 112) and calls
`do_downgrade(session)`
(line 113), currently a documented no-op (lines 86-96).
3. After `do_downgrade()` returns, `downgrade()` explicitly calls
`session.commit()` at
line 115, committing the database transaction from inside the revision
instead of leaving
transaction control to Alembic’s `context.begin_transaction()` in `env.py`.
4. If a subsequent downgrade step in the same Alembic-managed transaction
fails and
Alembic attempts to roll back, the manual commit has already finalized this
revision’s
effects, so downgrade behavior becomes desynchronized from Alembic’s
transactional
guarantees and rollback reliability now depends on this explicit commit
rather than the
configured per-migration transaction.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cc55daf17da1446f90f5ce2530050160&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=cc55daf17da1446f90f5ce2530050160&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/migrations/versions/2026-06-23_03-23_a7d3f1b9c2e4_cleanup_stale_can_import_pvm.py
**Line:** 115:115
**Comment:**
*Logic Error: The same explicit `session.commit()` in `downgrade()` has
the same transaction-boundary problem and can desynchronize downgrade behavior
from Alembic’s transaction handling; this should also remain under
Alembic-managed transaction control with flush-only ORM persistence.
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%2F39469&comment_hash=de7a172acd36d0a4ae31b4aeede99dd08fbac533eeae4b7325de7167c322cee1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39469&comment_hash=de7a172acd36d0a4ae31b4aeede99dd08fbac533eeae4b7325de7167c322cee1&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]
