Delete model artifacts (#5030)

Author: itallix

application/backend/app/api/dependencies.py

@@ -131,10 +131,11 @@ def get_system_service() -> SystemService:
 
 
 def get_model_service(
+    data_dir: Annotated[Path, Depends(get_data_dir)],
     db: Annotated[Session, Depends(get_db)],
 ) -> ModelService:
     """Provides a ModelService instance with the model reload event from the scheduler."""
-    return ModelService(db_session=db)
+    return ModelService(data_dir=data_dir, db_session=db)
 
 
 def get_webrtc_manager(request: Request) -> WebRTCManager:

application/backend/app/lifecycle.py

@@ -56,7 +56,7 @@ def setup_job_controller(data_dir: Path, max_parallel_jobs: int) -> tuple[JobQue
                 subset_service=SubsetService(),
                 subset_assigner=SubsetAssigner(),
                 dataset_service=DatasetService(data_dir=data_dir, label_service=LabelService()),
-                model_service=ModelService(),
+                model_service=ModelService(data_dir=data_dir),
                 training_configuration_service=TrainingConfigurationService(),
                 data_dir=data_dir,
                 db_session_factory=get_db_session,

application/backend/app/services/model_service.py

@@ -1,10 +1,13 @@
 # Copyright (C) 2025 Intel Corporation
 # SPDX-License-Identifier: Apache-2.0
 
+import shutil
 from dataclasses import dataclass
+from pathlib import Path
 from uuid import UUID
 
 from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm import Session
 
 from app.db.schema import ModelRevisionDB
 from app.models.training_configuration.configuration import TrainingConfiguration
@@ -31,6 +34,10 @@ class ModelRevisionMetadata:
 class ModelService(BaseSessionManagedService):
     """Service to register and activate models"""
 
+    def __init__(self, data_dir: Path, db_session: Session | None = None) -> None:
+        super().__init__(db_session)
+        self._projects_dir = data_dir / "projects"
+
     def get_model(self, project_id: UUID, model_id: UUID) -> ModelSchema:
         """
         Get a model.
@@ -56,6 +63,9 @@ def delete_model(self, project_id: UUID, model_id: UUID) -> None:
         """
         Delete a model.
 
+        Deletes a model revision from the database and deletes the folder from the filesystem
+        associated with this model.
+
         Args:
             project_id (UUID): The unique identifier of the project whose models to delete.
             model_id (UUID): The unique identifier of the model to delete.
@@ -69,8 +79,12 @@ def delete_model(self, project_id: UUID, model_id: UUID) -> None:
                 (e.g., the model is referenced by other entities).
         """
         model_rev_repo = ModelRevisionRepository(project_id=str(project_id), db=self.db_session)
+
+        path = self._projects_dir / str(project_id) / "models" / str(model_id)
+        if path.exists():
+            shutil.rmtree(path)
+
         try:
-            # TODO: delete model artifacts from filesystem when implemented
             deleted = model_rev_repo.delete(str(model_id))
             if not deleted:
                 raise ResourceNotFoundError(ResourceType.MODEL, str(model_id))

application/backend/tests/integration/services/test_model_service.py

@@ -1,5 +1,6 @@
 # Copyright (C) 2025 Intel Corporation
 # SPDX-License-Identifier: Apache-2.0
+from pathlib import Path
 from uuid import UUID, uuid4
 
 import pytest
@@ -28,9 +29,9 @@ def setup_project_with_models(
 
 
 @pytest.fixture
-def fxt_model_service(db_session: Session) -> ModelService:
+def fxt_model_service(tmp_path: Path, db_session: Session) -> ModelService:
     """Fixture to create a ModelService instance."""
-    return ModelService(db_session=db_session)
+    return ModelService(data_dir=tmp_path, db_session=db_session)
 
 
 class TestModelServiceIntegration:
@@ -65,10 +66,63 @@ def test_non_existent_model(self, model_operation, fxt_project_id, fxt_db_projec
         assert excinfo.value.resource_type == ResourceType.MODEL
         assert excinfo.value.resource_id == str(model_id)
 
-    def test_delete_model(
-        self, fxt_project_id: UUID, fxt_model_id: UUID, fxt_model_service: ModelService, db_session: Session
+    def test_delete_model_no_files(
+        self,
+        tmp_path: Path,
+        fxt_project_id: UUID,
+        fxt_model_id: UUID,
+        fxt_model_service: ModelService,
+        db_session: Session,
     ):
-        """Test deleting a model by ID."""
+        """Test that deleting a model removes its database record when no filesystem artifacts exist."""
+        model_rev_path = tmp_path / "projects" / str(fxt_project_id) / "models" / str(fxt_model_id)
+
         fxt_model_service.delete_model(project_id=fxt_project_id, model_id=fxt_model_id)
 
         assert db_session.get(ModelRevisionDB, str(fxt_model_id)) is None
+        assert not model_rev_path.exists()
+
+    def test_delete_model_with_files(
+        self,
+        tmp_path: Path,
+        fxt_project_id: UUID,
+        fxt_model_id: UUID,
+        fxt_model_service: ModelService,
+        db_session: Session,
+    ):
+        """Test that deleting a model removes both its database record and filesystem artifacts."""
+        model_rev_path = tmp_path / "projects" / str(fxt_project_id) / "models" / str(fxt_model_id)
+        model_rev_path.mkdir(parents=True, exist_ok=True)
+        (model_rev_path / "config.yaml").touch()
+        (model_rev_path / "training.log").touch()
+
+        fxt_model_service.delete_model(project_id=fxt_project_id, model_id=fxt_model_id)
+
+        assert db_session.get(ModelRevisionDB, str(fxt_model_id)) is None
+        assert not model_rev_path.exists()
+
+    def test_delete_model_with_files_no_permission(
+        self,
+        tmp_path: Path,
+        fxt_project_id: UUID,
+        fxt_model_id: UUID,
+        fxt_model_service: ModelService,
+        db_session: Session,
+    ):
+        """Test that deleting a model removes both its database record and filesystem artifacts."""
+        model_rev_path = tmp_path / "projects" / str(fxt_project_id) / "models" / str(fxt_model_id)
+        model_rev_path.mkdir(parents=True, exist_ok=True)
+        (model_rev_path / "config.yaml").touch()
+
+        # Make directory read-only to simulate permission error
+        model_rev_path.chmod(0o444)
+
+        try:
+            with pytest.raises(OSError):
+                fxt_model_service.delete_model(project_id=fxt_project_id, model_id=fxt_model_id)
+
+            assert db_session.get(ModelRevisionDB, str(fxt_model_id)) is not None
+            assert model_rev_path.exists()
+        finally:
+            # Cleanup: restore permissions so pytest can clean up temp directory
+            model_rev_path.chmod(0o755)