Ianmacleod/add model delete (#261)

* starting work on model.delete()

* adding server implementation

* adding client changes to delete based on id, not name of model

* changing use case to be based on name, not id

* adding client model changes

* adding unit tests

* updating tests

* .

* .

* fix

* fix

* pls work

* addressing feedback from review

* reverting accidental change to conftest

* add new test

Author: ian-scale

clients/python/llmengine/model.py

@@ -334,7 +334,7 @@ def list(cls) -> ListLLMEndpointsResponse:
         return ListLLMEndpointsResponse.parse_obj(response)
 
     @classmethod
-    def delete(cls, model: str) -> DeleteLLMEndpointResponse:
+    def delete(cls, model_endpoint_name: str) -> DeleteLLMEndpointResponse:
         """
         Deletes an LLM model.
 
@@ -345,11 +345,11 @@ def delete(cls, model: str) -> DeleteLLMEndpointResponse:
         Engine, an error will be thrown.
 
         Args:
-            model (`str`):
-                Name of the model
+            model_endpoint_name (`str`):
+                Name of the model endpoint to be deleted
 
         Returns:
-            response: whether the model was successfully deleted
+            response: whether the model endpoint was successfully deleted
 
         === "Deleting model in Python"
             ```python
@@ -366,7 +366,9 @@ def delete(cls, model: str) -> DeleteLLMEndpointResponse:
             }
             ```
         """
-        response = cls._delete(f"v1/llm/model-endpoints/{model}", timeout=DEFAULT_TIMEOUT)
+        response = cls._delete(
+            f"v1/llm/model-endpoints/{model_endpoint_name}", timeout=DEFAULT_TIMEOUT
+        )
         return DeleteLLMEndpointResponse.parse_obj(response)
 
     @classmethod

model-engine/model_engine_server/api/llms_v1.py

@@ -21,6 +21,7 @@
     CreateFineTuneResponse,
     CreateLLMModelEndpointV1Request,
     CreateLLMModelEndpointV1Response,
+    DeleteLLMEndpointResponse,
     GetFineTuneEventsResponse,
     GetFineTuneResponse,
     GetLLMModelEndpointV1Response,
@@ -40,9 +41,11 @@
 )
 from model_engine_server.core.loggers import filename_wo_ext, make_logger
 from model_engine_server.domain.exceptions import (
+    EndpointDeleteFailedException,
     EndpointLabelsException,
     EndpointResourceInvalidRequestException,
     EndpointUnsupportedInferenceTypeException,
+    ExistingEndpointOperationInProgressException,
     InvalidRequestException,
     LLMFineTuningMethodNotImplementedException,
     LLMFineTuningQuotaReached,
@@ -59,6 +62,7 @@
     CompletionStreamV1UseCase,
     CompletionSyncV1UseCase,
     CreateLLMModelEndpointV1UseCase,
+    DeleteLLMEndpointByNameUseCase,
     GetLLMModelEndpointByNameV1UseCase,
     ListLLMModelEndpointsV1UseCase,
     ModelDownloadV1UseCase,
@@ -384,3 +388,41 @@ async def download_model_endpoint(
             status_code=404,
             detail="The requested fine-tuned model could not be found.",
         ) from exc
+
+
+@llm_router_v1.delete(
+    "/model-endpoints/{model_endpoint_name}", response_model=DeleteLLMEndpointResponse
+)
+async def delete_llm_model_endpoint(
+    model_endpoint_name: str,
+    auth: User = Depends(verify_authentication),
+    external_interfaces: ExternalInterfaces = Depends(get_external_interfaces),
+) -> DeleteLLMEndpointResponse:
+    add_trace_resource_name("llm_model_endpoints_delete")
+    logger.info(f"DELETE /model-endpoints/{model_endpoint_name} for {auth}")
+    try:
+        use_case = DeleteLLMEndpointByNameUseCase(
+            llm_model_endpoint_service=external_interfaces.llm_model_endpoint_service,
+            model_endpoint_service=external_interfaces.model_endpoint_service,
+        )
+        return await use_case.execute(user=auth, model_endpoint_name=model_endpoint_name)
+    except (ObjectNotFoundException) as exc:
+        raise HTTPException(
+            status_code=404,
+            detail="The requested model endpoint could not be found.",
+        ) from exc
+    except (ObjectNotAuthorizedException) as exc:
+        raise HTTPException(
+            status_code=403,
+            detail="You don't have permission to delete the requested model endpoint.",
+        ) from exc
+    except ExistingEndpointOperationInProgressException as exc:
+        raise HTTPException(
+            status_code=409,
+            detail="Existing operation on endpoint in progress, try again later.",
+        ) from exc
+    except EndpointDeleteFailedException as exc:  # pragma: no cover
+        raise HTTPException(
+            status_code=500,
+            detail="deletion of endpoint failed.",
+        ) from exc

model-engine/model_engine_server/common/dtos/llms.py

@@ -228,3 +228,7 @@ class ModelDownloadResponse(BaseModel):
     urls: Dict[str, str] = Field(
         ..., description="Dictionary of (file_name, url) pairs to download the model from."
     )
+
+
+class DeleteLLMEndpointResponse(BaseModel):
+    deleted: bool

model-engine/model_engine_server/domain/use_cases/llm_model_endpoint_use_cases.py

@@ -21,6 +21,7 @@
     CompletionSyncV1Response,
     CreateLLMModelEndpointV1Request,
     CreateLLMModelEndpointV1Response,
+    DeleteLLMEndpointResponse,
     GetLLMModelEndpointV1Response,
     ListLLMModelEndpointsV1Response,
     ModelDownloadRequest,
@@ -779,8 +780,45 @@ async def execute(self, user: User, model_endpoint_name: str) -> GetLLMModelEndp
         return _model_endpoint_entity_to_get_llm_model_endpoint_response(model_endpoint)
 
 
-class DeleteLLMModelEndpointByIdV1UseCase:
-    pass
+class DeleteLLMEndpointByNameUseCase:
+    """
+    Use case for deleting an LLM Model Endpoint of a given user by endpoint name.
+    """
+
+    def __init__(
+        self,
+        model_endpoint_service: ModelEndpointService,
+        llm_model_endpoint_service: LLMModelEndpointService,
+    ):
+        self.model_endpoint_service = model_endpoint_service
+        self.llm_model_endpoint_service = llm_model_endpoint_service
+        self.authz_module = LiveAuthorizationModule()
+
+    async def execute(self, user: User, model_endpoint_name: str) -> DeleteLLMEndpointResponse:
+        """
+        Runs the use case to delete the LLM endpoint owned by the user with the given name.
+
+        Args:
+            user: The owner of the model endpoint.
+            model_endpoint_name: The name of the model endpoint.
+
+        Returns:
+            A response object that contains a boolean indicating if deletion was successful.
+
+        Raises:
+            ObjectNotFoundException: If a model endpoint with the given name could not be found.
+            ObjectNotAuthorizedException: If the owner does not own the model endpoint.
+        """
+        model_endpoints = await self.llm_model_endpoint_service.list_llm_model_endpoints(
+            owner=user.user_id, name=model_endpoint_name, order_by=None
+        )
+        if len(model_endpoints) != 1:
+            raise ObjectNotFoundException
+        model_endpoint = model_endpoints[0]
+        if not self.authz_module.check_access_write_owned_entity(user, model_endpoint.record):
+            raise ObjectNotAuthorizedException
+        await self.model_endpoint_service.delete_model_endpoint(model_endpoint.record.id)
+        return DeleteLLMEndpointResponse(deleted=True)
 
 
 def deepspeed_result_to_tokens(result: Dict[str, Any]) -> List[TokenOutput]:

model-engine/tests/unit/domain/test_llm_use_cases.py

@@ -35,6 +35,7 @@
     CompletionStreamV1UseCase,
     CompletionSyncV1UseCase,
     CreateLLMModelEndpointV1UseCase,
+    DeleteLLMEndpointByNameUseCase,
     GetLLMModelEndpointByNameV1UseCase,
     ModelDownloadV1UseCase,
 )
@@ -869,3 +870,88 @@ async def test_download_nonexistent_model_raises_not_found(
     )
     with pytest.raises(ObjectNotFoundException):
         await use_case.execute(user=user, request=request)
+
+
+@pytest.mark.asyncio
+async def test_delete_model_success(
+    fake_model_endpoint_service,
+    fake_llm_model_endpoint_service,
+    llm_model_endpoint_sync: Tuple[ModelEndpoint, Any],
+    test_api_key: str,
+):
+    fake_llm_model_endpoint_service.add_model_endpoint(llm_model_endpoint_sync[0])
+    fake_model_endpoint_service.add_model_endpoint(llm_model_endpoint_sync[0])
+    use_case = DeleteLLMEndpointByNameUseCase(
+        model_endpoint_service=fake_model_endpoint_service,
+        llm_model_endpoint_service=fake_llm_model_endpoint_service,
+    )
+    user = User(user_id=test_api_key, team_id=test_api_key, is_privileged_user=True)
+    response = await use_case.execute(
+        user=user, model_endpoint_name=llm_model_endpoint_sync[0].record.name
+    )
+    remaining_endpoint_model_service = await fake_model_endpoint_service.get_model_endpoint(
+        llm_model_endpoint_sync[0].record.id
+    )
+    assert remaining_endpoint_model_service is None
+    assert response.deleted is True
+
+
+@pytest.mark.asyncio
+async def test_delete_nonexistent_model_raises_not_found(
+    fake_model_endpoint_service,
+    fake_llm_model_endpoint_service,
+    llm_model_endpoint_sync: Tuple[ModelEndpoint, Any],
+    test_api_key: str,
+):
+    fake_llm_model_endpoint_service.add_model_endpoint(llm_model_endpoint_sync[0])
+    fake_model_endpoint_service.add_model_endpoint(llm_model_endpoint_sync[0])
+    use_case = DeleteLLMEndpointByNameUseCase(
+        model_endpoint_service=fake_model_endpoint_service,
+        llm_model_endpoint_service=fake_llm_model_endpoint_service,
+    )
+    user = User(user_id=test_api_key, team_id=test_api_key, is_privileged_user=True)
+    with pytest.raises(ObjectNotFoundException):
+        await use_case.execute(user=user, model_endpoint_name="nonexistent-model")
+
+
+@pytest.mark.asyncio
+async def test_delete_unauthorized_model_raises_not_authorized(
+    fake_model_endpoint_service,
+    fake_llm_model_endpoint_service,
+    llm_model_endpoint_sync: Tuple[ModelEndpoint, Any],
+):
+    fake_llm_model_endpoint_service.add_model_endpoint(llm_model_endpoint_sync[0])
+    fake_model_endpoint_service.add_model_endpoint(llm_model_endpoint_sync[0])
+    use_case = DeleteLLMEndpointByNameUseCase(
+        model_endpoint_service=fake_model_endpoint_service,
+        llm_model_endpoint_service=fake_llm_model_endpoint_service,
+    )
+    user = User(user_id="fakeapikey", team_id="fakeapikey", is_privileged_user=True)
+    with pytest.raises(ObjectNotAuthorizedException):
+        await use_case.execute(
+            user=user, model_endpoint_name=llm_model_endpoint_sync[0].record.name
+        )
+
+
+@pytest.mark.asyncio
+async def test_delete_public_inference_model_raises_not_authorized(
+    fake_model_endpoint_service,
+    fake_llm_model_endpoint_service,
+    llm_model_endpoint_sync: Tuple[ModelEndpoint, Any],
+    test_api_key,
+):
+    fake_llm_model_endpoint_service.add_model_endpoint(llm_model_endpoint_sync[0])
+    fake_model_endpoint_service.add_model_endpoint(llm_model_endpoint_sync[0])
+    use_case = DeleteLLMEndpointByNameUseCase(
+        model_endpoint_service=fake_model_endpoint_service,
+        llm_model_endpoint_service=fake_llm_model_endpoint_service,
+    )
+    user = User(
+        user_id="fakeapikey", team_id="faketeam", is_privileged_user=True
+    )  # write access is based on team_id, so team_id != owner's team_id
+    with pytest.raises(
+        ObjectNotAuthorizedException
+    ):  # user cannot delete public inference model they don't own
+        await use_case.execute(
+            user=user, model_endpoint_name=llm_model_endpoint_sync[0].record.name
+        )