uranusjr commented on code in PR #33017:
URL: https://github.com/apache/airflow/pull/33017#discussion_r1286646301


##########
airflow/operators/python.py:
##########
@@ -528,6 +537,9 @@ def __init__(
         templates_exts: list[str] | None = None,
         expect_airflow: bool = True,
         skip_on_exit_code: int | Container[int] | None = None,
+        index_urls: None | Collection[str] | str = None,
+        cache_venv: bool = False,
+        cache_path: str = "/tmp",

Review Comment:
   Why not make this `cache_path: str | None = None` and disable cache on None?
   
   Also a change of the argument name may be a good idea; it’s unclear from the 
current name what exactly is cached since pip and Python (on execution time) 
also has their own cache setup.



##########
airflow/operators/python.py:
##########
@@ -528,6 +537,9 @@ def __init__(
         templates_exts: list[str] | None = None,
         expect_airflow: bool = True,
         skip_on_exit_code: int | Container[int] | None = None,
+        index_urls: None | Collection[str] | str = None,
+        cache_venv: bool = False,
+        cache_path: str = "/tmp",

Review Comment:
   Why not make this `cache_path: str | None = None` and disable cache on None?
   
   Also a change of the argument name may be a good idea; it’s unclear from the 
current name what exactly is cached since pip and Python (on execution time) 
also has their own cache setup.



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