Merge pull request #2141 from fractal-analytics-platform/2133-expose-…

…more-flexible-for-pip-cache 2133 expose more flexible for pip cache
fractal-analytics-platform · Dec 16, 2024 · e86e72d · e86e72d
2 parents da6b7e4 + 9177816
commit e86e72d
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,9 +2,11 @@
 
 # Unreleased
 
-* Tasks lifecycle:
-    * Prevent deactivation of task groups with `"github.com"` in pip-freeze information (\#2144).
 
+* App:
+    * Add `FRACTAL_PIP_CACHE_DIR` configuration variable (\#2141).
+* Tasks life cycle:
+    * Prevent deactivation of task groups with `"github.com"` in pip-freeze information (\#2144).
 * Runner:
     * Handle early shutdown for sudo SLURM executor (\#2132).
     * Fix repeated setting of `timestamp_ended` in task-group reactivation (\#2140).

diff --git a/fractal_server/config.py b/fractal_server/config.py
@@ -492,6 +492,39 @@ def check_tasks_python(cls, values) -> None:
     Whether to include the v1 API.
     """
 
+    FRACTAL_PIP_CACHE_DIR: Optional[str] = None
+    """
+    Absolute path to the cache directory for `pip`; if unset,
+    `--no-cache-dir` is used.
+    """
+
+    @validator("FRACTAL_PIP_CACHE_DIR", always=True)
+    def absolute_FRACTAL_PIP_CACHE_DIR(cls, v):
+        """
+        If `FRACTAL_PIP_CACHE_DIR` is a relative path, fail.
+        """
+        if v is None:
+            return None
+        elif not Path(v).is_absolute():
+            raise FractalConfigurationError(
+                f"Non-absolute value for FRACTAL_PIP_CACHE_DIR={v}"
+            )
+        else:
+            return v
+
+    @property
+    def PIP_CACHE_DIR_ARG(self) -> str:
+        """
+        Option for `pip install`, based on `FRACTAL_PIP_CACHE_DIR` value.
+
+        If `FRACTAL_PIP_CACHE_DIR` is set, then return
+        `--cache-dir /somewhere`; else return `--no-cache-dir`.
+        """
+        if self.FRACTAL_PIP_CACHE_DIR is not None:
+            return f"--cache-dir {self.FRACTAL_PIP_CACHE_DIR}"
+        else:
+            return "--no-cache-dir"
+
     FRACTAL_MAX_PIP_VERSION: str = "24.0"
     """
     Maximum value at which to update `pip` before performing task collection.
@@ -538,15 +571,13 @@ def check_db(self) -> None:
             raise FractalConfigurationError("POSTGRES_DB cannot be None.")
 
     def check_runner(self) -> None:
-
         if not self.FRACTAL_RUNNER_WORKING_BASE_DIR:
             raise FractalConfigurationError(
                 "FRACTAL_RUNNER_WORKING_BASE_DIR cannot be None."
             )
 
         info = f"FRACTAL_RUNNER_BACKEND={self.FRACTAL_RUNNER_BACKEND}"
         if self.FRACTAL_RUNNER_BACKEND == "slurm":
-
             from fractal_server.app.runner.executors.slurm._slurm_config import (  # noqa: E501
                 load_slurm_config_file,
             )

diff --git a/fractal_server/tasks/v2/templates/2_pip_install.sh b/fractal_server/tasks/v2/templates/2_pip_install.sh
@@ -10,15 +10,16 @@ PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__
 INSTALL_STRING=__INSTALL_STRING__
 PINNED_PACKAGE_LIST="__PINNED_PACKAGE_LIST__"
 FRACTAL_MAX_PIP_VERSION="__FRACTAL_MAX_PIP_VERSION__"
+FRACTAL_PIP_CACHE_DIR_ARG="__FRACTAL_PIP_CACHE_DIR_ARG__"
 
 TIME_START=$(date +%s)
 
 VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python
 
 # Upgrade `pip` and install `setuptools`
 write_log "START upgrade pip and install setuptools"
-"$VENVPYTHON" -m pip install --no-cache-dir "pip<=${FRACTAL_MAX_PIP_VERSION}" --upgrade
-"$VENVPYTHON" -m pip install --no-cache-dir setuptools
+"$VENVPYTHON" -m pip install ${FRACTAL_PIP_CACHE_DIR_ARG} "pip<=${FRACTAL_MAX_PIP_VERSION}" --upgrade
+"$VENVPYTHON" -m pip install ${FRACTAL_PIP_CACHE_DIR_ARG} setuptools
 write_log "END   upgrade pip and install setuptools"
 echo
 
@@ -41,7 +42,7 @@ if [ "$PINNED_PACKAGE_LIST" != "" ]; then
     done
 
     write_log "All packages in ${PINNED_PACKAGE_LIST} are already installed, proceed with specific versions."
-    "$VENVPYTHON" -m pip install --no-cache-dir "$PINNED_PACKAGE_LIST"
+    "$VENVPYTHON" -m pip install ${FRACTAL_PIP_CACHE_DIR_ARG} "$PINNED_PACKAGE_LIST"
     write_log "END installing pinned versions $PINNED_PACKAGE_LIST"
 else
     write_log "SKIP installing pinned versions $PINNED_PACKAGE_LIST (empty list)"

diff --git a/fractal_server/tasks/v2/templates/6_pip_install_from_freeze.sh b/fractal_server/tasks/v2/templates/6_pip_install_from_freeze.sh
@@ -9,21 +9,22 @@ write_log(){
 PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__
 PIP_FREEZE_FILE=__PIP_FREEZE_FILE__
 FRACTAL_MAX_PIP_VERSION=__FRACTAL_MAX_PIP_VERSION__
+FRACTAL_PIP_CACHE_DIR_ARG="__FRACTAL_PIP_CACHE_DIR_ARG__"
 
 TIME_START=$(date +%s)
 
 VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python
 
 # Upgrade `pip` and install `setuptools`
 write_log "START upgrade pip and install setuptools"
-"$VENVPYTHON" -m pip install --no-cache-dir "pip<=${FRACTAL_MAX_PIP_VERSION}" --upgrade
-"$VENVPYTHON" -m pip install --no-cache-dir setuptools
+"$VENVPYTHON" -m pip install ${FRACTAL_PIP_CACHE_DIR_ARG} "pip<=${FRACTAL_MAX_PIP_VERSION}" --upgrade
+"$VENVPYTHON" -m pip install ${FRACTAL_PIP_CACHE_DIR_ARG} setuptools
 write_log "END   upgrade pip and install setuptools"
 echo
 
 # Install from pip-freeze file
 write_log "START installing requirements from ${PIP_FREEZE_FILE}"
-"$VENVPYTHON" -m pip install -r "${PIP_FREEZE_FILE}"
+"$VENVPYTHON" -m pip install ${FRACTAL_PIP_CACHE_DIR_ARG} -r "${PIP_FREEZE_FILE}"
 write_log "END   installing requirements from ${PIP_FREEZE_FILE}"
 echo
 

diff --git a/fractal_server/tasks/v2/utils_templates.py b/fractal_server/tasks/v2/utils_templates.py
@@ -81,6 +81,7 @@ def get_collection_replacements(
             "__FRACTAL_MAX_PIP_VERSION__",
             settings.FRACTAL_MAX_PIP_VERSION,
         ),
+        ("__FRACTAL_PIP_CACHE_DIR_ARG__", settings.PIP_CACHE_DIR_ARG),
         (
             "__PINNED_PACKAGE_LIST__",
             task_group.pinned_package_versions_string,

diff --git a/tests/no_version/test_unit_config.py b/tests/no_version/test_unit_config.py
@@ -198,7 +198,6 @@ def test_settings_injection(override_settings):
 def test_settings_check(
     settings_dict: dict[str, str], raises: bool, testdata_path: Path
 ):
-
     debug(settings_dict, raises)
 
     # Workaround to set FRACTAL_SLURM_CONFIG_FILE to a valid path, which
@@ -223,7 +222,6 @@ def test_settings_check(
 
 
 def test_settings_check_wrong_python():
-
     # Create a Settings instance
     with pytest.raises(FractalConfigurationError) as e:
         Settings(
@@ -270,8 +268,42 @@ def test_make_FRACTAL_RUNNER_WORKING_BASE_DIR_absolute():
     assert settings.FRACTAL_RUNNER_WORKING_BASE_DIR.is_absolute()
 
 
-def test_OAuthClientConfig():
+def test_FRACTAL_PIP_CACHE_DIR():
+    """
+    Test `Settings.pip_cache_dir` & absolute_FRACTAL_PIP_CACHE_DIR validator.
+    """
+
+    SOME_DIR = "/some/dir"
+
+    assert (
+        Settings(
+            JWT_SECRET_KEY="secret",
+            POSTGRES_DB="db-name",
+            FRACTAL_RUNNER_WORKING_BASE_DIR="relative-path",
+            FRACTAL_PIP_CACHE_DIR=SOME_DIR,
+        ).PIP_CACHE_DIR_ARG
+        == f"--cache-dir {SOME_DIR}"
+    )
+
+    assert (
+        Settings(
+            JWT_SECRET_KEY="secret",
+            POSTGRES_DB="db-name",
+            FRACTAL_RUNNER_WORKING_BASE_DIR="relative-path",
+        ).PIP_CACHE_DIR_ARG
+        == "--no-cache-dir"
+    )
+
+    with pytest.raises(FractalConfigurationError):
+        Settings(
+            JWT_SECRET_KEY="secret",
+            POSTGRES_DB="db-name",
+            FRACTAL_RUNNER_WORKING_BASE_DIR="relative-path",
+            FRACTAL_PIP_CACHE_DIR="~/CACHE_DIR",
+        )
+
 
+def test_OAuthClientConfig():
     config = OAuthClientConfig(
         CLIENT_NAME="GOOGLE",
         CLIENT_ID="123",

diff --git a/tests/v2/06_tasks/test_unit_bash_scripts.py b/tests/v2/06_tasks/test_unit_bash_scripts.py
@@ -1,5 +1,6 @@
 import pytest
 
+from fractal_server.config import Settings
 from fractal_server.tasks.v2.local.collect import _customize_and_run_template
 from fractal_server.tasks.v2.utils_templates import customize_template
 from fractal_server.tasks.v2.utils_templates import (
@@ -57,7 +58,14 @@ def test_template_1(tmp_path, current_py_version):
     assert venv_path.exists()
 
 
-def test_template_2(tmp_path, testdata_path, current_py_version):
+def test_template_2(
+    tmp_path, testdata_path, current_py_version, override_settings_factory
+):
+    settings = Settings(
+        **Settings(
+            FRACTAL_PIP_CACHE_DIR=(tmp_path / "CACHE_DIR").as_posix()
+        ).dict()
+    )
     path = tmp_path / "unit_templates"
     venv_path = path / "venv"
     install_string = testdata_path.parent / (
@@ -73,6 +81,7 @@ def test_template_2(tmp_path, testdata_path, current_py_version):
         ("__INSTALL_STRING__", install_string.as_posix()),
         ("__PINNED_PACKAGE_LIST__", pinned_pkg_list),
         ("__FRACTAL_MAX_PIP_VERSION__", "99"),
+        ("__FRACTAL_PIP_CACHE_DIR_ARG__", settings.PIP_CACHE_DIR_ARG),
     ]
     script_path = tmp_path / "2_good.sh"
     customize_template(
@@ -95,6 +104,7 @@ def test_template_2(tmp_path, testdata_path, current_py_version):
         ("__INSTALL_STRING__", install_string.as_posix()),
         ("__PINNED_PACKAGE_LIST__", pinned_pkg_list),
         ("__FRACTAL_MAX_PIP_VERSION__", "25"),
+        ("__FRACTAL_PIP_CACHE_DIR_ARG__", Settings().PIP_CACHE_DIR_ARG),
     ]
     script_path = tmp_path / "2_bad_pkg.sh"
     customize_template(
@@ -176,7 +186,6 @@ def _parse_pip_freeze_output(stdout: str) -> dict[str, str]:
 
 
 def test_templates_freeze(tmp_path, current_py_version):
-
     # Create two venvs
     venv_path_1 = tmp_path / "venv1"
     venv_path_2 = tmp_path / "venv2"
@@ -198,6 +207,10 @@ def test_templates_freeze(tmp_path, current_py_version):
                 ("__INSTALL_STRING__", "pip"),
                 ("__FRACTAL_MAX_PIP_VERSION__", "99"),
                 ("__PINNED_PACKAGE_LIST__", ""),
+                (
+                    "__FRACTAL_PIP_CACHE_DIR_ARG__",
+                    Settings().PIP_CACHE_DIR_ARG,
+                ),
             ],
             script_dir=tmp_path,
             logger_name=__name__,
@@ -212,6 +225,7 @@ def test_templates_freeze(tmp_path, current_py_version):
             ("__INSTALL_STRING__", "devtools"),
             ("__FRACTAL_MAX_PIP_VERSION__", "99"),
             ("__PINNED_PACKAGE_LIST__", ""),
+            ("__FRACTAL_PIP_CACHE_DIR_ARG__", Settings().PIP_CACHE_DIR_ARG),
         ],
         script_dir=tmp_path,
         logger_name=__name__,
@@ -240,6 +254,7 @@ def test_templates_freeze(tmp_path, current_py_version):
             ("__PACKAGE_ENV_DIR__", venv_path_2.as_posix()),
             ("__PIP_FREEZE_FILE__", requirements_file.as_posix()),
             ("__FRACTAL_MAX_PIP_VERSION__", "99"),
+            ("__FRACTAL_PIP_CACHE_DIR_ARG__", Settings().PIP_CACHE_DIR_ARG),
         ],
         script_dir=tmp_path,
         logger_name=__name__,
@@ -260,7 +275,6 @@ def test_templates_freeze(tmp_path, current_py_version):
 
 
 def test_venv_size_and_file_number(tmp_path):
-
     # Create folders
     folder = tmp_path / "test"
     subfolder = folder / "subfolder"