Merge pull request #2121 from fractal-analytics-platform/1992-remove-…

…cache_dir-and-use-project_dirfractal_cache

Remove `UserSettings.cache_dir`

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 # Unreleased
 
+* API
+    * Remove `cache_dir` and use `project_dir/.fractal_cache` (\#2121).
 * Docs
     * Improve docstrings and reduce mkdocs warnings (\#2122).
 

fractal_server/app/models/security.py

@@ -76,7 +76,6 @@ class UserOAuth(SQLModel, table=True):
         is_verified:
         slurm_user:
         slurm_accounts:
-        cache_dir:
         username:
         oauth_accounts:
     """

fractal_server/app/models/user_settings.py

@@ -20,7 +20,7 @@ class UserSettings(SQLModel, table=True):
         ssh_tasks_dir: Task-venvs base folder on `ssh_host`.
         ssh_jobs_dir: Jobs base folder on `ssh_host`.
         slurm_user: Local user, to be impersonated via `sudo -u`
-        cache_dir: Folder where `slurm_user` can write.
+        project_dir: Folder where `slurm_user` can write.
     """
 
     __tablename__ = "user_settings"
@@ -35,5 +35,4 @@ class UserSettings(SQLModel, table=True):
     ssh_tasks_dir: Optional[str] = None
     ssh_jobs_dir: Optional[str] = None
     slurm_user: Optional[str] = None
-    cache_dir: Optional[str] = None
     project_dir: Optional[str] = None

fractal_server/app/routes/api/v1/project.py

@@ -448,6 +448,12 @@ async def apply_workflow(
     await db.commit()
     await db.refresh(job)
 
+    cache_dir = (
+        f"{user_settings.project_dir}/.fractal_cache"
+        if user_settings.project_dir is not None
+        else None
+    )
+
     background_tasks.add_task(
         submit_workflow,
         workflow_id=workflow.id,
@@ -456,7 +462,7 @@ async def apply_workflow(
         job_id=job.id,
         worker_init=apply_workflow.worker_init,
         slurm_user=user_settings.slurm_user,
-        user_cache_dir=user_settings.cache_dir,
+        user_cache_dir=cache_dir,
     )
     request.app.state.jobsV1.append(job.id)
     logger.info(

fractal_server/app/routes/api/v2/submit.py

@@ -186,18 +186,22 @@ async def apply_workflow(
         f"_{timestamp_string}"
     )
 
+    cache_dir = (
+        Path(user_settings.project_dir) / ".fractal_cache"
+        if user_settings.project_dir is not None
+        else None
+    )
+
     # Define user-side job directory
     if FRACTAL_RUNNER_BACKEND == "local":
         WORKFLOW_DIR_REMOTE = WORKFLOW_DIR_LOCAL
     elif FRACTAL_RUNNER_BACKEND == "local_experimental":
         WORKFLOW_DIR_REMOTE = WORKFLOW_DIR_LOCAL
     elif FRACTAL_RUNNER_BACKEND == "slurm":
-        WORKFLOW_DIR_REMOTE = (
-            Path(user_settings.cache_dir) / f"{WORKFLOW_DIR_LOCAL.name}"
-        )
+        WORKFLOW_DIR_REMOTE = cache_dir / WORKFLOW_DIR_LOCAL.name
     elif FRACTAL_RUNNER_BACKEND == "slurm_ssh":
         WORKFLOW_DIR_REMOTE = (
-            Path(user_settings.ssh_jobs_dir) / f"{WORKFLOW_DIR_LOCAL.name}"
+            Path(user_settings.ssh_jobs_dir) / WORKFLOW_DIR_LOCAL.name
         )
 
     # Update job folders in the db
@@ -229,7 +233,7 @@ async def apply_workflow(
         user_settings=user_settings,
         worker_init=job.worker_init,
         slurm_user=user_settings.slurm_user,
-        user_cache_dir=user_settings.cache_dir,
+        user_cache_dir=cache_dir.as_posix() if cache_dir else None,
         fractal_ssh=fractal_ssh,
     )
     request.app.state.jobsV2.append(job.id)

fractal_server/app/schemas/user_settings.py

@@ -31,14 +31,12 @@ class UserSettingsRead(BaseModel):
     ssh_jobs_dir: Optional[str] = None
     slurm_user: Optional[str] = None
     slurm_accounts: list[str]
-    cache_dir: Optional[str] = None
     project_dir: Optional[str] = None
 
 
 class UserSettingsReadStrict(BaseModel):
     slurm_user: Optional[str] = None
     slurm_accounts: list[str]
-    cache_dir: Optional[str] = None
     ssh_username: Optional[str] = None
     project_dir: Optional[str] = None
 
@@ -55,7 +53,6 @@ class UserSettingsUpdate(BaseModel, extra=Extra.forbid):
     ssh_jobs_dir: Optional[str] = None
     slurm_user: Optional[str] = None
     slurm_accounts: Optional[list[StrictStr]] = None
-    cache_dir: Optional[str] = None
     project_dir: Optional[str] = None
 
     _ssh_host = validator("ssh_host", allow_reuse=True)(
@@ -87,13 +84,6 @@ def slurm_accounts_validator(cls, value):
             value[i] = valstr(f"slurm_accounts[{i}]")(item)
         return val_unique_list("slurm_accounts")(value)
 
-    @validator("cache_dir")
-    def cache_dir_validator(cls, value):
-        if value is None:
-            return None
-        validate_cmd(value)
-        return val_absolute_path("cache_dir")(value)
-
     @validator("project_dir")
     def project_dir_validator(cls, value):
         if value is None:
@@ -104,15 +94,7 @@ def project_dir_validator(cls, value):
 
 class UserSettingsUpdateStrict(BaseModel, extra=Extra.forbid):
     slurm_accounts: Optional[list[StrictStr]] = None
-    cache_dir: Optional[str] = None
 
     _slurm_accounts = validator("slurm_accounts", allow_reuse=True)(
         val_unique_list("slurm_accounts")
     )
-
-    @validator("cache_dir")
-    def cache_dir_validator(cls, value):
-        if value is None:
-            return value
-        validate_cmd(value)
-        return val_absolute_path("cache_dir")(value)

fractal_server/app/user_settings.py

@@ -32,11 +32,11 @@ class SlurmSudoUserSettings(BaseModel):
 
     Attributes:
         slurm_user: User to be impersonated via `sudo -u`.
-        cache_dir: Folder where `slurm_user` can write.
+        project_dir: Folder where `slurm_user` can write.
         slurm_accounts:
             List of SLURM accounts, to be used upon Fractal job submission.
     """
 
     slurm_user: str
-    cache_dir: str
+    project_dir: str
     slurm_accounts: list[str]

fractal_server/migrations/versions/316140ff7ee1_remove_usersettings_cache_dir.py

@@ -0,0 +1,36 @@
+"""Remove UserSettings.cache_dir
+
+Revision ID: 316140ff7ee1
+Revises: d256a7379ab8
+Create Date: 2024-12-03 10:15:53.255958
+
+"""
+import sqlalchemy as sa
+from alembic import op
+
+
+# revision identifiers, used by Alembic.
+revision = "316140ff7ee1"
+down_revision = "d256a7379ab8"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.batch_alter_table("user_settings", schema=None) as batch_op:
+        batch_op.drop_column("cache_dir")
+
+    # ### end Alembic commands ###
+
+
+def downgrade() -> None:
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.batch_alter_table("user_settings", schema=None) as batch_op:
+        batch_op.add_column(
+            sa.Column(
+                "cache_dir", sa.VARCHAR(), autoincrement=False, nullable=True
+            )
+        )
+
+    # ### end Alembic commands ###

tests/no_version/test_api_current_user.py

@@ -100,9 +100,7 @@ async def test_patch_current_user_no_extra(registered_client):
     """
     res = await registered_client.patch(PREFIX, json={})
     assert res.status_code == 200
-    res = await registered_client.patch(
-        PREFIX, json={"cache_dir": "/tmp", "foo": "bar"}
-    )
+    res = await registered_client.patch(PREFIX, json={"foo": "bar"})
     assert res.status_code == 422
 
 
@@ -178,7 +176,7 @@ async def test_get_and_patch_current_user_settings(registered_client):
         else:
             assert v is None
 
-    patch = dict(slurm_accounts=["foo", "bar"], cache_dir="/tmp/foo_cache")
+    patch = dict(slurm_accounts=["foo", "bar"])
     res = await registered_client.patch(f"{PREFIX}settings/", json=patch)
     assert res.status_code == 200
 

tests/no_version/test_api_user_groups.py

@@ -332,7 +332,6 @@ async def test_patch_user_settings_bulk(
             ssh_jobs_dir=None,
             slurm_user="test01",
             slurm_accounts=[],
-            cache_dir=None,
             project_dir=None,
         ) == user.settings.dict(exclude={"id"})
 
@@ -353,7 +352,6 @@ async def test_patch_user_settings_bulk(
         ssh_jobs_dir="/tmp/job",
         # missing `slurm_user`
         slurm_accounts=["foo", "bar"],
-        cache_dir="/tmp/cache",
         project_dir="/foo",
     )
     res = await registered_superuser_client.patch(
@@ -376,7 +374,6 @@ async def test_patch_user_settings_bulk(
         ssh_jobs_dir=None,
         slurm_user="test01",
         slurm_accounts=[],
-        cache_dir=None,
         project_dir=None,
     ) == user4.settings.dict(exclude={"id"})
 

tests/no_version/test_api_users.py

@@ -437,7 +437,6 @@ async def test_get_and_patch_user_settings(registered_superuser_client):
         # missing "ssh_jobs_dir"
         slurm_user="fractal",
         slurm_accounts=["foo", "bar"],
-        cache_dir="/tmp/cache",
     )
     res = await registered_superuser_client.patch(
         f"{PREFIX}/users/{user_id}/settings/", json=patch

tests/no_version/test_fixture_MockCurrentUser.py

@@ -2,22 +2,19 @@
 from devtools import debug
 
 
-@pytest.mark.parametrize("cache_dir", ("/some/path", None))
 @pytest.mark.parametrize("username", ("my_username", None))
 @pytest.mark.parametrize("slurm_user", ("test01", None))
 async def test_MockCurrentUser_fixture(
     MockCurrentUser,
-    cache_dir,
     username,
     slurm_user,
 ):
 
     user_kwargs = dict(username=username)
-    user_settings_dict = dict(cache_dir=cache_dir, slurm_user=slurm_user)
+    user_settings_dict = dict(slurm_user=slurm_user)
     async with MockCurrentUser(
         user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
     ) as user:
         debug(user)
         assert user.username == username
-        assert user.settings.cache_dir == cache_dir
         assert user.settings.slurm_user == slurm_user

tests/no_version/test_unit_schemas_no_version.py

@@ -114,24 +114,12 @@ def test_user_settings_update():
     )
     assert update_request_body.slurm_accounts is None
 
-    update_request_body = UserSettingsUpdate(cache_dir=None)
-    assert update_request_body.cache_dir is None
-    assert "cache_dir" in update_request_body.dict(exclude_unset=True).keys()
-
-    update_request_body = UserSettingsUpdateStrict(cache_dir=None)
-    assert update_request_body.cache_dir is None
-    assert "cache_dir" in update_request_body.dict(exclude_unset=True).keys()
-
     with pytest.raises(ValidationError):
         UserSettingsUpdate(slurm_accounts=[""])
     with pytest.raises(ValidationError):
         UserSettingsUpdate(slurm_accounts=["a", "a"])
     with pytest.raises(ValidationError):
         UserSettingsUpdate(slurm_accounts=["a", "a "])
-    with pytest.raises(ValidationError):
-        UserSettingsUpdate(cache_dir="non/absolute/path")
-    with pytest.raises(ValidationError):
-        UserSettingsUpdate(cache_dir="/invalid;command; $(ls)")
     with pytest.raises(ValidationError):
         UserSettingsUpdateStrict(ssh_host="NEW_HOST")
     with pytest.raises(ValidationError):
@@ -145,23 +133,13 @@ def test_user_settings_update():
         "ssh_tasks_dir",
         "ssh_jobs_dir",
         "slurm_user",
-        "cache_dir",
-    ]
-    nullable_attributes_strict = [
-        "cache_dir",
     ]
     for key in nullable_attributes:
         update_request_body = UserSettingsUpdate(**{key: None})
         assert getattr(update_request_body, key) is None
         assert key in update_request_body.dict(exclude_unset=True)
         assert key not in update_request_body.dict(exclude_none=True)
 
-    for key in nullable_attributes_strict:
-        update_request_body = UserSettingsUpdateStrict(**{key: None})
-        assert getattr(update_request_body, key) is None
-        assert key in update_request_body.dict(exclude_unset=True)
-        assert key not in update_request_body.dict(exclude_none=True)
-
 
 def test_unit_val_absolute_path():
     val_absolute_path("this_attr")("/path")

tests/v1/04_api/test_project_apply_api.py

@@ -260,16 +260,16 @@ async def test_project_apply_missing_user_attributes(
 ):
     """
     When using the slurm backend, `user.settings.slurm_user` and
-    `user.settings.cache_dir` become required attributes.
+    `user.settings.project_dir` become required attributes.
     If they are missing, the apply endpoint fails with a 422 error.
     """
 
     override_settings_factory(FRACTAL_RUNNER_BACKEND="slurm")
 
     async with MockCurrentUser(user_kwargs=dict(is_verified=True)) as user:
-        # Make sure that user.settings.cache_dir was not set
+        # Make sure that user.settings.project_dir was not set
         debug(user)
-        assert user.settings.cache_dir is None
+        assert user.settings.project_dir is None
 
         # Create project, datasets, workflow, task, workflowtask
         project = await project_factory(user)
@@ -303,7 +303,7 @@ async def test_project_apply_missing_user_attributes(
         assert res.status_code == 422
         assert "User settings are not valid" in res.json()["detail"]
 
-        user.settings.cache_dir = "/tmp"
+        user.settings.project_dir = "/tmp"
         user.settings.slurm_user = None
         await db.merge(user)
         await db.commit()

tests/v1/07_full_workflow/test_full_workflow.py

@@ -63,7 +63,7 @@ async def test_full_workflow(
         request.getfixturevalue("relink_python_interpreter_v1")
         user_cache_dir = str(tmp777_path / f"user_cache_dir-{backend}")
 
-        user_settings_dict["cache_dir"] = user_cache_dir
+        user_settings_dict["project_dir"] = user_cache_dir
         user_settings_dict["slurm_user"] = SLURM_USER
 
     async with MockCurrentUser(
@@ -274,7 +274,7 @@ async def test_failing_workflow_UnknownError(
         request.getfixturevalue("monkey_slurm")
         request.getfixturevalue("relink_python_interpreter_v1")
         user_cache_dir = str(tmp777_path / f"user_cache_dir-{backend}")
-        user_settings_dict["cache_dir"] = user_cache_dir
+        user_settings_dict["project_dir"] = user_cache_dir
         user_settings_dict["slurm_user"] = SLURM_USER
 
     async with MockCurrentUser(
@@ -373,7 +373,7 @@ async def test_failing_workflow_TaskExecutionError(
         request.getfixturevalue("monkey_slurm")
         request.getfixturevalue("relink_python_interpreter_v1")
         user_cache_dir = str(tmp777_path / f"user_cache_dir-{backend}")
-        user_settings_dict["cache_dir"] = user_cache_dir
+        user_settings_dict["project_dir"] = user_cache_dir
         user_settings_dict["slurm_user"] = SLURM_USER
 
     async with MockCurrentUser(
@@ -533,7 +533,9 @@ async def test_failing_workflow_JobExecutionError_slurm(
 
     user_cache_dir = str(tmp777_path / "user_cache_dir")
     user_kwargs = dict(is_verified=True)
-    user_settings_dict = dict(cache_dir=user_cache_dir, slurm_user=SLURM_USER)
+    user_settings_dict = dict(
+        project_dir=user_cache_dir, slurm_user=SLURM_USER
+    )
     async with MockCurrentUser(
         user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
     ) as user:
@@ -920,7 +922,7 @@ async def test_non_executable_task_command(
         request.getfixturevalue("monkey_slurm")
         request.getfixturevalue("relink_python_interpreter_v1")
         user_cache_dir = str(tmp777_path / f"user_cache_dir-{backend}")
-        user_settings_dict["cache_dir"] = user_cache_dir
+        user_settings_dict["project_dir"] = user_cache_dir
         user_settings_dict["slurm_user"] = SLURM_USER
 
     async with MockCurrentUser(