[GitHub] [airflow] potiuk commented on a diff in pull request #33527: Use `NOT EXISTS` subquery instead of `tuple_not_in_condition`

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk commented on code in PR #33527:
URL: https://github.com/apache/airflow/pull/33527#discussion_r1299835539


##########
airflow/models/renderedtifields.py:
##########
@@ -201,11 +209,11 @@ def delete_old_records(
         :param num_to_keep: Number of Records to keep
         :param session: SqlAlchemy Session
         """
-        from airflow.models.dagrun import DagRun
-
         if num_to_keep <= 0:
             return
 
+        from airflow.models.dagrun import DagRun

Review Comment:
   Side comment. I've learned recently to use more and more local imports. I think basically that when not really needed, the advice of "use top level imports by default" is less and less relevant for "heavy" imports. 
   
   The lazy import PEP is years away and it does not seem that it will be enabled by default - you will have to specify an option when you launch interpreter, and I think it's simply inevitable that people will use more and more local imports and the tooling (static checks IDE support etc.) will start supporting it better. 
   
   That's my bet actually that rather than switching to lazy impors we will switch to "local imports for heavy things by default".  You can see it all over Airflow's code already. 
   
   The only reason why you would like to use top-level imports for "heavy" packages is when you use the same import in multiple methods in the same module - but this also can be solved by simply local importing that partiular module. 
   
   I bet some tooling for "detect heavy top-level imports" will soon show up in one of the static check tooling.



-- 
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-unsubscribe@airflow.apache.org

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