Prab-27 commented on issue #59402:
URL: https://github.com/apache/airflow/issues/59402#issuecomment-3654356544

   > [@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 
[#54843](https://github.com/apache/airflow/pull/54843) 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?
   
   Your concern is correct. Earlier, the script detected only session.query(), 
but at some point the session was used as ss. or in a similar way, and then the 
script was modified to delete this query‑related PR. : #54843
   
   Now I’m thinking of removing that part from the issue where .query() is used 
as another method from other objects, and I’ll remove it after re‑implementing 
the script as it was previously


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