From 67a87c5f33d5fbc8e85625b1af960234b969a8a4 Mon Sep 17 00:00:00 2001
From: Mattt <mattt@replicate.com>
Date: Wed, 25 Sep 2024 12:31:17 -0700
Subject: [PATCH] Consistently return Boolean for delete methods (#359)

Resolves #356

When deleting a model, Replicate's API returns with a 204 and an empty
response body. The client library fails to decode this and raises and
exception.

This PR updates the `replicate.models.delete` method to return `bool`
instead to indicate success. It also harmonizes the other delete methods
for deployments and files to do the same.

Signed-off-by: Mattt Zmuda <mattt@replicate.com>
---
 replicate/deployment.py | 10 ++++++----
 replicate/file.py       | 10 ++++++----
 replicate/model.py      | 29 +++++++++++++----------------
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/replicate/deployment.py b/replicate/deployment.py
index 1f0fdaba..b1adf542 100644
--- a/replicate/deployment.py
+++ b/replicate/deployment.py
@@ -358,7 +358,7 @@ async def async_update(
 
         return _json_to_deployment(self._client, resp.json())
 
-    def delete(self, deployment_owner: str, deployment_name: str) -> None:
+    def delete(self, deployment_owner: str, deployment_name: str) -> bool:
         """
         Delete an existing deployment.
 
@@ -366,12 +366,13 @@ def delete(self, deployment_owner: str, deployment_name: str) -> None:
             deployment_owner: The owner of the deployment.
             deployment_name: The name of the deployment.
         """
-        self._client._request(
+        resp = self._client._request(
             "DELETE",
             f"/v1/deployments/{deployment_owner}/{deployment_name}",
         )
+        return resp.status_code == 204
 
-    async def async_delete(self, deployment_owner: str, deployment_name: str) -> None:
+    async def async_delete(self, deployment_owner: str, deployment_name: str) -> bool:
         """
         Delete an existing deployment asynchronously.
 
@@ -379,10 +380,11 @@ async def async_delete(self, deployment_owner: str, deployment_name: str) -> Non
             deployment_owner: The owner of the deployment.
             deployment_name: The name of the deployment.
         """
-        await self._client._async_request(
+        resp = await self._client._async_request(
             "DELETE",
             f"/v1/deployments/{deployment_owner}/{deployment_name}",
         )
+        return resp.status_code == 204
 
     @property
     def predictions(self) -> "DeploymentsPredictions":
diff --git a/replicate/file.py b/replicate/file.py
index 53d9ea76..bb3ff86b 100644
--- a/replicate/file.py
+++ b/replicate/file.py
@@ -133,15 +133,17 @@ async def async_list(self) -> List[File]:
         resp = await self._client._async_request("GET", "/v1/files")
         return [_json_to_file(obj) for obj in resp.json().get("results", [])]
 
-    def delete(self, file_id: str) -> None:
+    def delete(self, file_id: str) -> bool:
         """Delete an uploaded file by its ID."""
 
-        _ = self._client._request("DELETE", f"/v1/files/{file_id}")
+        resp = self._client._request("DELETE", f"/v1/files/{file_id}")
+        return resp.status_code == 204
 
-    async def async_delete(self, file_id: str) -> None:
+    async def async_delete(self, file_id: str) -> bool:
         """Delete an uploaded file by its ID asynchronously."""
 
-        _ = await self._client._async_request("DELETE", f"/v1/files/{file_id}")
+        resp = await self._client._async_request("DELETE", f"/v1/files/{file_id}")
+        return resp.status_code == 204
 
 
 def _create_file_params(
diff --git a/replicate/model.py b/replicate/model.py
index 31f625af..1cf144aa 100644
--- a/replicate/model.py
+++ b/replicate/model.py
@@ -287,41 +287,38 @@ async def async_get(self, *args, **kwargs) -> Model:
         return _json_to_model(self._client, resp.json())
 
     @overload
-    def delete(self, key: str) -> Model: ...
+    def delete(self, key: str) -> bool: ...
 
     @overload
-    def delete(self, owner: str, name: str) -> Model: ...
+    def delete(self, owner: str, name: str) -> bool: ...
 
-    def delete(self, *args, **kwargs) -> Model:
+    def delete(self, *args, **kwargs) -> bool:
         """
         Delete a model by name.
-        """
 
+        Returns:
+            `True` if deletion was successful, otherwise `False`.
+        """
         url = _delete_model_url(*args, **kwargs)
         resp = self._client._request("DELETE", url)
-
-        return _json_to_model(self._client, resp.json())
+        return resp.status_code == 204
 
     @overload
-    async def async_delete(self, key: str) -> Model: ...
+    async def async_delete(self, key: str) -> bool: ...
 
     @overload
-    async def async_delete(self, owner: str, name: str) -> Model: ...
+    async def async_delete(self, owner: str, name: str) -> bool: ...
 
-    async def async_delete(self, *args, **kwargs) -> Model:
+    async def async_delete(self, *args, **kwargs) -> bool:
         """
-        Delete a model by name.
+        Asynchronously delete a model by name.
 
-        Args:
-            key: The qualified name of the model, in the format `owner/name`.
         Returns:
-            The model.
+            `True` if deletion was successful, otherwise `False`.
         """
-
         url = _delete_model_url(*args, **kwargs)
         resp = await self._client._async_request("DELETE", url)
-
-        return _json_to_model(self._client, resp.json())
+        return resp.status_code == 204
 
     class CreateModelParams(TypedDict):
         """Parameters for creating a model."""