henry3260 commented on issue #59402:
URL: https://github.com/apache/airflow/issues/59402#issuecomment-3654006356

   > [@Prab-27](https://github.com/Prab-27) I’ve been sending a few PRs, and 
out of curiosity I took a closer look at the internal logic. I think there 
might be one small issue worth mentioning.
   > 
   > The current logic at lines 39–41 treats all .query() method calls as 
deprecated:
   > 
   > 
[airflow/scripts/ci/prek/prevent_deprecated_sqlalchemy_usage.py](https://github.com/apache/airflow/blob/8a109dbcb11f2edaa6255f543b0a8d37466cb439/scripts/ci/prek/prevent_deprecated_sqlalchemy_usage.py#L35-L53)
   > 
   > Lines 35 to 53 in 
[8a109db](/apache/airflow/commit/8a109dbcb11f2edaa6255f543b0a8d37466cb439)
   > 
   >  def check_session_query(mod: ast.Module, file_path: str) -> bool: 
   >      errors = False 
   >      for node in ast.walk(mod): 
   >          if isinstance(node, ast.Call) and isinstance(node.func, 
ast.Attribute): 
   >              if node.func.attr == "query": 
   >                  console.print(f"Deprecated query-obj found at line 
{node.lineno} in {file_path}.") 
   >                  errors = True 
   >          if isinstance(node, ast.ImportFrom): 
   >              if ( 
   >                  node.module == "sqlalchemy.orm.query" 
   >                  or node.module == "sqlalchemy" 
   >                  or node.module == "sqlalchemy.orm" 
   >              ): 
   >                  for alias in node.names: 
   >                      if alias.name == "Query": 
   >                          console.print(f"Deprecated Query class found at 
line {node.lineno} in {file_path}.") 
   >                          errors = True 
   >   
   >      return errors 
   > This may potentially flag some legitimate API method calls used in 
provider code, such as those in Google or AWS providers.
   > 
   > In those cases, .query() is not SQLAlchemy’s deprecated session.query(), 
but a valid .query() method defined on a different object. WDYT?
   
   Great catch! I suggest we refine the AST check to inspect the caller of the 
.query() method.
   
   In Airflow, SQLAlchemy usage is predominantly session.query(...). We can 
modify the script to verify if the object calling .query is explicitly named 
session


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

Reply via email to