Deprecate make_results in favour of get_results (#17)

* Breaking change: remove wait argument from make_results

* deprecation warning

* add test

Author: malmans2

datapi/api_client.py

@@ -334,7 +334,7 @@ def get_remote(self, request_id: str) -> datapi.Remote:
         -------
         datapi.Remote
         """
-        return self._retrieve_api.get_job(request_id).make_remote()
+        return self._retrieve_api.get_job(request_id).get_remote()
 
     def get_results(self, request_id: str) -> datapi.Results:
         """
@@ -349,7 +349,7 @@ def get_results(self, request_id: str) -> datapi.Results:
         -------
         datapi.Results
         """
-        return self.get_remote(request_id).make_results()
+        return self.get_remote(request_id).get_results()
 
     def retrieve(
         self,
@@ -410,7 +410,7 @@ def submit_and_wait_on_results(
         -------
         datapi.Results
         """
-        return self._retrieve_api.submit(collection_id, request).make_results()
+        return self._retrieve_api.submit(collection_id, request).get_results()
 
     def unstar_collection(self, collection_id: str) -> None:
         return self._profile_api.unstar_collection(collection_id)

datapi/processing.py

@@ -320,7 +320,7 @@ def submit(self, request: dict[str, Any]) -> datapi.Remote:
             json={"inputs": request},
             **self._request_kwargs,
         )
-        return job.make_remote()
+        return job.get_remote()
 
     def apply_constraints(self, request: dict[str, Any]) -> dict[str, Any]:
         """Apply constraints to the parameters in a request.
@@ -486,13 +486,13 @@ def results_ready(self) -> bool:
         if status in ("accepted", "running"):
             return False
         if status == "failed":
-            results = self.make_results(wait=False)
+            results = self._make_results(wait=False)
             raise ProcessingFailedError(error_json_to_message(results._json_dict))
         if status in ("dismissed", "deleted"):
             raise ProcessingFailedError(f"API state {status!r}")
         raise ProcessingFailedError(f"Unknown API state {status!r}")
 
-    def make_results(self, wait: bool = True) -> Results:
+    def _make_results(self, wait: bool) -> Results:
         if wait:
             self._wait_on_results()
         response = self._get_api_response("get")
@@ -503,6 +503,24 @@ def make_results(self, wait: bool = True) -> Results:
         results = Results.from_request("get", results_url, **self._request_kwargs)
         return results
 
+    def make_results(self, wait: bool = True) -> Results:
+        warnings.warn(
+            "`make_results` has been deprecated, and in the future will raise an error."
+            "Please use `get_results` from now on.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self._make_results(wait)
+
+    def get_results(self) -> Results:
+        """Retrieve results.
+
+        Returns
+        -------
+        datapi.Results
+        """
+        return self._make_results(wait=True)
+
     def download(self, target: str | None = None) -> str:
         """Download the results.
 
@@ -516,7 +534,7 @@ def download(self, target: str | None = None) -> str:
         str
             Path to the retrieved file.
         """
-        results = self.make_results()
+        results = self.get_results()
         return results.download(target)
 
     def delete(self) -> dict[str, Any]:
@@ -562,7 +580,7 @@ def reply(self) -> dict[str, Any]:
         elif reply["state"] == "failed":
             reply.setdefault("error", {})
             try:
-                self.make_results()
+                self.get_results()
             except Exception as exc:
                 reply["error"].setdefault("message", str(exc))
 
@@ -594,13 +612,22 @@ def __del__(self) -> None:
 
 @attrs.define
 class Job(ApiResponse):
-    def make_remote(self) -> Remote:
+    def get_remote(self) -> Remote:
         if self.response.request.method == "POST":
             url = self._get_link_href(rel="monitor")
         else:
             url = self._get_link_href(rel="self")
         return Remote(url, **self._request_kwargs)
 
+    def make_remote(self) -> Remote:
+        warnings.warn(
+            "`make_remote` has been deprecated, and in the future will raise an error."
+            "Please use `get_remote` from now on.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.get_remote()
+
 
 @attrs.define
 class Jobs(ApiResponsePaginated):

tests/integration_test_20_processing.py

@@ -62,7 +62,7 @@ def test_processing_get_jobs_status(api_anon_client: ApiClient) -> None:
     remote = api_anon_client.submit("test-adaptor-dummy", {"format": "foo"})
     request_id = remote.request_id
     with pytest.raises(HTTPError, match="400 Client Error: Bad Request"):
-        remote.make_results()
+        remote.get_results()
     assert request_id in api_anon_client.get_jobs(status="failed").request_ids
     assert request_id not in api_anon_client.get_jobs(status="successful").request_ids
 

tests/integration_test_30_remote.py

@@ -101,7 +101,13 @@ def test_remote_datetimes(api_anon_client: ApiClient) -> None:
     assert isinstance(remote.creation_datetime, datetime.datetime)
     assert remote.end_datetime is None
 
-    remote.make_results()
+    remote.get_results()
     assert remote.start_datetime is not None
     assert remote.end_datetime is not None
     assert remote.creation_datetime < remote.start_datetime < remote.end_datetime
+
+
+def test_make_results_deprecation(api_anon_client: ApiClient) -> None:
+    remote = api_anon_client.submit("test-adaptor-dummy", {})
+    with pytest.warns(DeprecationWarning, match="`make_results` has been deprecated"):
+        remote.make_results()

tests/integration_test_70_legacy_api_client.py

@@ -246,12 +246,12 @@ def test_legacy_api_client_download(
     target = client.download(remote)
     assert os.path.getsize(target) == 1
 
-    results = remote.make_results()
+    results = remote.get_results()
     results_dict = {
         "location": results.location,
         "contentLength": results.content_length,
     }
-    results_tuple = (remote, remote.make_results(), results_dict)
+    results_tuple = (remote, remote.get_results(), results_dict)
     target1 = "remote.grib"
     target2 = "results.grib"
     target3 = "dict.grib"