potiuk commented on code in PR #22695: URL: https://github.com/apache/airflow/pull/22695#discussion_r841476559
########## dev/breeze/src/airflow_breeze/utils/run_utils.py: ########## @@ -102,6 +99,9 @@ def check_package_installed(package_name: str) -> bool: def get_filesystem_type(filepath): + # We import it locally so that click autocomplete works + import psutil Review Comment: Because when autocomplete loads list of methods, it imports `breeze.py` and it has to import it fully to determine list of available options (quite similarly like Airflow parsing dag files to determine the structure of DAG. Therefore we should make sure that very little number of packages need to be imported. There are two reasons: * in the environment where autocomplete is executed, psutil might be simply not installed (And import will fail) * importing packages takes time - we want to make autocomplete as fast as possible so that it returns list of available options immediately This is the same thing as the one described by @blag here: https://github.com/apache/airflow/pull/22613#issuecomment-1086788871 and why we need to do the same in Airflow when we convert to click + click-rich. It's also fundamentally the same case as the one described here in Airflow docs for DAG top-level code: https://airflow.apache.org/docs/apache-airflow/stable/best-practices.html?highlight=best%20practices#top-level-python-code -- 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: dev-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
