bito-code-review[bot] commented on code in PR #43273:
URL: https://github.com/apache/superset/pull/43273#discussion_r3799410309
##########
superset/db_engine_specs/gsheets.py:
##########
@@ -389,7 +389,6 @@ def validate_parameters(
}
}
},
- future=True,
)
conn = engine.connect()
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Connection resource leak</b></div>
<div id="fix">
The connection at line 393 is never explicitly closed and has no `finally`
block or context manager. Multiple early returns (lines 414, 425) bypass any
potential cleanup. Compare to the correct pattern at line 525: `with
engine.connect() as conn:`. This causes a resource leak where the database
connection remains open after the function returns.
</div>
</div>
<small><i>Code Review Run #d015f4</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/migrations/versions/2024-02-07_17-13_87d38ad83218_migrate_can_view_and_drill_permission.py:
##########
@@ -73,7 +73,7 @@ def upgrade():
def downgrade():
bind = op.get_bind()
- session = Session(bind=bind, future=True)
+ session = Session(bind=bind)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inconsistent exception handling</b></div>
<div id="fix">
The `downgrade()` function catches SQLAlchemyError but silently ignores it
by printing and calling `pass`, allowing the migration to report success even
on failure. The `upgrade()` function correctly re-raises exceptions.
Inconsistent error handling between upgrade/downgrade creates a maintenance
hazard and can mask database migration failures.
</div>
</div>
<small><i>Code Review Run #d015f4</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
