Re: [PR] fix: use local db model in migration [superset]

Wed, 27 Aug 2025 17:36:55 -0700 


korbit-ai[bot] commented on code in PR #34875:
URL: https://github.com/apache/superset/pull/34875#discussion_r2305714405


##########
superset/migrations/shared/catalogs.py:
##########
@@ -27,21 +27,46 @@
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import Session
 
-from superset import db, security_manager
+# Note: Import Database functionality without importing the actual model
+from superset import db, db_engine_specs, security_manager
+from superset.databases.utils import make_url_safe
 from superset.db_engine_specs.base import GenericDBException
 from superset.migrations.shared.security_converge import (
     add_pvms,
     Permission,
     PermissionView,
     ViewMenu,
 )
-from superset.models.core import Database
 
 logger = logging.getLogger("alembic.env")
 
 Base: Type[Any] = declarative_base()
 
 
+class Database(Base):
+    """Local Database model for migration"""
+
+    __tablename__ = "dbs"
+
+    id = sa.Column(sa.Integer, primary_key=True)
+    sqlalchemy_uri = sa.Column(sa.String(1024))
+    encrypted_extra = sa.Column(sa.Text)
+
+    @property
+    def db_engine_spec(self) -> Type[Any]:
+        url = make_url_safe(self.sqlalchemy_uri)
+        backend = url.get_backend_name()
+        try:
+            driver = url.get_driver_name()
+        except Exception:
+            driver = None

Review Comment:
   ### Overly broad exception handler in database driver detection 
<sub>![category Security](https://img.shields.io/badge/Security-e11d48)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Overly broad exception handling that catches all exceptions silently and 
continues execution.
   
   
   ###### Why this matters
   Silent failure in driver name extraction could lead to incorrect database 
driver selection, potentially causing security bypass or unintended database 
access.
   
   ###### Suggested change ∙ *Feature Preview*
   ```python
   try:
       driver = url.get_driver_name()
   except (AttributeError, KeyError) as e:
       logger.warning(f"Failed to get database driver name: {e}")
       driver = None
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9941e4f5-cd4d-4d7c-a8ea-3616b37ccc73/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9941e4f5-cd4d-4d7c-a8ea-3616b37ccc73?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9941e4f5-cd4d-4d7c-a8ea-3616b37ccc73?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9941e4f5-cd4d-4d7c-a8ea-3616b37ccc73?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9941e4f5-cd4d-4d7c-a8ea-3616b37ccc73)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:d0d22578-be95-4c8f-aa62-50a2fefb5a97 -->
   
   
   [](d0d22578-be95-4c8f-aa62-50a2fefb5a97)



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