uranusjr commented on code in PR #33355:
URL: https://github.com/apache/airflow/pull/33355#discussion_r1323884252
##########
airflow/operators/python.py:
##########
@@ -606,7 +614,60 @@ def _prepare_venv(self, venv_path: Path) -> None:
index_urls=self.index_urls,
)
+ def _calculate_cache_hash(self) -> str:
Review Comment:
By ache validation I meant deciding whether a new task should use an
existing environment or create a new one. And instead of trying to encode this
as a string in the environment’s path, I feel it’d be easier to just dump the
information into JSON, and compare that JSON directly, which is more reliable,
likely cheaper, and easier to for post mortem debugging since that file
conveniently capture what parameters the environment was created with. Since
the implementation already uses a marker file, it can be used to store the JSON
as well. This frees us from having to use a pre-determined directory name,
which can open up some possible future improvements (I know there are people
out there who feel very strongly on the directory name for some reason).
##########
airflow/operators/python.py:
##########
@@ -606,7 +614,60 @@ def _prepare_venv(self, venv_path: Path) -> None:
index_urls=self.index_urls,
)
+ def _calculate_cache_hash(self) -> str:
Review Comment:
By ache validation I meant deciding whether a new task should use an
existing environment or create a new one. And instead of trying to encode this
as a string in the environment’s path, I feel it’d be easier to just dump the
information into JSON, and compare that JSON directly, which is more reliable,
likely cheaper, and easier to for post mortem debugging since that file
conveniently capture what parameters the environment was created with. Since
the implementation already uses a marker file, it can be used to store the JSON
as well. This frees us from having to use a pre-determined directory name,
which can open up some possible future improvements (I know there are people
out there who feel very strongly on the directory name for some reason).
--
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]
