Dev-iL commented on code in PR #37545:
URL: https://github.com/apache/airflow/pull/37545#discussion_r1495648821


##########
airflow/utils/orm_event_handlers.py:
##########
@@ -23,18 +23,25 @@
 import traceback
 
 import sqlalchemy.orm.mapper
-from sqlalchemy import event, exc
+from sqlalchemy import __version__ as sqlalchemy_version, event, exc
 
 from airflow.configuration import conf
 
 log = logging.getLogger(__name__)
 
+SQL_ALCHEMY_V1 = sqlalchemy_version.startswith("1")
+
 
 def setup_event_handlers(engine):
     """Setups event handlers."""
     from airflow.models import import_all_models
 
-    event.listen(sqlalchemy.orm.mapper, "before_configured", 
import_all_models, once=True)
+    event.listen(
+        sqlalchemy.orm.mapper if SQL_ALCHEMY_V1 else sqlalchemy.orm.Mapper,

Review Comment:
   @potiuk Do you not want to avoid code duplication? Why is it preferable to 
copy this entire block while changing one line over refactoring the version 
checking to a separate utility where the library and target version are inputs, 
then invoke from everywhere as needed?
   ```python
   def is_pydantic_2_installed() -> bool:
       import sys
   
       from packaging.version import Version
   
       if sys.version_info >= (3, 9):
           from importlib.metadata import distribution
       else:
           from importlib_metadata import distribution
       try:
           return Version(distribution("pydantic").version) >= Version("2.0.0")
       except ImportError:
           return False
   ```



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to