codeant-ai-for-open-source[bot] commented on code in PR #41980:
URL: https://github.com/apache/superset/pull/41980#discussion_r3611300233


##########
superset/utils/core.py:
##########
@@ -641,9 +641,7 @@ def generic_find_constraint_name(
     table: str, columns: set[str], referenced: str, database: SQLAlchemy
 ) -> str | None:
     """Utility to find a constraint name in alembic migrations"""
-    tbl = sa.Table(
-        table, database.metadata, autoload=True, autoload_with=database.engine
-    )
+    tbl = sa.Table(table, database.metadata, autoload_with=database.engine)

Review Comment:
   Short answer: **probably not a regression**.
   
   This change is primarily a **SQLAlchemy 2.0 compatibility refactor**:
   
   - `autoload=True` is deprecated, so removing it is the right direction.
   - `autoload_with=...` is the supported replacement.
   - The bot’s concern about `database.metadata` is **real**, but it points to 
a **pre-existing reflection pattern**, not something newly introduced by this 
diff.
   
   So for this specific hunk, the issue is more accurately described as:
   
   > “We’re cleaning up deprecated reflection usage, but this helper still 
relies on shared metadata, which can be risky in migrations.”
   
   If you want to make it fully robust, the minimal fix would be to reflect 
into a fresh `MetaData()` instead of `database.metadata`, e.g.:
   
   ```python
   metadata = sa.MetaData()
   tbl = sa.Table(table, metadata, autoload_with=database.engine)
   ```
   
   But if the question is strictly whether **this PR change** is a regression: 
**no, it’s a compatibility refactor, not a behavioral regression introduced by 
the removal of `autoload=True`.**
   
   If you'd like, I can also review the rest of the comments on this PR and 
separate the ones that are true regressions from the ones that are just 
cleanup/compatibility fixes.



-- 
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]

Reply via email to