copy_result_tb: return the response to the PUT request

copy_result_tb returns the response from the server. The callers
are responsible for interpreting it.

push.py: construct a reasonable message out of the response and check
if the HTTP status is "OK" (i.e < 400). We exit with 0 if so; we
really expect only two "OK" cases: 200 is a duplicate PUT request (we
print a message on stderr in this case) and 201 is success.
If the HTTP status indicates error (>= 400), we output the message
on stderr and exit with 1.

move.py is not touched by this PR: it will be dealt with in a future
PR.

lib/pbench/agent/results.py

@@ -313,7 +313,7 @@ def __init__(
 
     def copy_result_tb(
         self, token: str, access: Optional[str] = None, metadata: Optional[List] = None
-    ) -> None:
+    ) -> requests.Response:
         """Copies the tar ball from the agent to the configured server using upload
         API.
 
@@ -327,6 +327,9 @@ def copy_result_tb(
             metadata: list of metadata keys to be sent on PUT. (Optional)
                 Format: key:value
 
+        Returns:
+            response from the PUT request
+
         Raises:
             RuntimeError     if a connection to the server fails
             FileUploadError  if the tar ball failed to upload properly
@@ -367,17 +370,12 @@ def copy_result_tb(
                     )
                 )
 
-                response = requests.Session().send(request)
-                response.raise_for_status()
-                self.logger.info("File uploaded successfully")
+                return requests.Session().send(request)
             except requests.exceptions.ConnectionError as exc:
                 raise RuntimeError(f"Cannot connect to '{self.upload_url}': {exc}")
             except Exception as exc:
                 raise self.FileUploadError(
-                    "There was something wrong with file upload request: "
+                    "There was something wrong with the file upload request: "
                     f"file: '{self.tarball}', URL: '{self.upload_url}';"
                     f" error: '{exc}'"
                 )
-        assert (
-            response.ok
-        ), f"Logic error!  Unexpected error response, '{response.reason}' ({response.status_code})"

lib/pbench/cli/agent/commands/results/push.py

@@ -2,6 +2,7 @@
 from typing import List
 
 import click
+import requests
 
 from pbench.agent.base import BaseCommand
 from pbench.agent.results import CopyResultTb
@@ -24,10 +25,25 @@ def execute(self) -> int:
             self.config,
             self.logger,
         )
-        crt.copy_result_tb(
+        res = crt.copy_result_tb(
             self.context.token, self.context.access, self.context.metadata
         )
-        return 0
+
+        try:
+            msg = res.json()["message"]
+        except requests.exceptions.JSONDecodeError:
+            msg = res.text if res.text else res.reason
+
+        if res.ok:
+            if res.status_code == 200:
+                click.echo(f"{msg}", err=True)
+            return 0
+        else:
+            click.echo(
+                f"HTTP Error status: {res.status_code}, message: {msg}",
+                err=True,
+            )
+            return 1
 
 
 @sort_click_command_parameters

lib/pbench/test/unit/agent/task/test_results_push.py

@@ -23,11 +23,12 @@ class TestResultsPush:
     URL = "http://pbench.example.com/api/v1"
 
     @staticmethod
-    def add_http_mock_response(code: HTTPStatus = HTTPStatus.OK):
+    def add_http_mock_response(code: HTTPStatus = HTTPStatus.OK, message: str = ""):
         responses.add(
             responses.PUT,
             f"{TestResultsPush.URL}/upload/{os.path.basename(tarball)}",
             status=code,
+            json={"message": message},
         )
 
     @staticmethod
@@ -148,7 +149,7 @@ def test_multiple_meta_args_single_option():
             ],
         )
         assert result.exit_code == 0, result.stderr
-        assert result.stderr == "File uploaded successfully\n"
+        assert result.stdout == ""
 
     @staticmethod
     @responses.activate
@@ -172,7 +173,56 @@ def test_multiple_meta_args():
             ],
         )
         assert result.exit_code == 0, result.stderr
-        assert result.stderr == "File uploaded successfully\n"
+        assert result.stdout == ""
+
+    @staticmethod
+    @responses.activate
+    def test_normal_created():
+        """Test normal operation when all arguments and options are specified"""
+
+        TestResultsPush.add_http_mock_response(code=HTTPStatus.CREATED)
+        runner = CliRunner(mix_stderr=False)
+        result = runner.invoke(
+            main,
+            args=[
+                TestResultsPush.TOKN_SWITCH,
+                TestResultsPush.TOKN_TEXT,
+                TestResultsPush.ACCESS_SWITCH,
+                TestResultsPush.ACCESS_TEXT,
+                TestResultsPush.META_SWITCH,
+                TestResultsPush.META_TEXT_TEST + "," + TestResultsPush.META_TEXT_FOO,
+                tarball,
+            ],
+        )
+        assert result.exit_code == 0, result.stderr
+        assert result.stdout == ""
+
+    @staticmethod
+    @responses.activate
+    def test_error_too_large_tarball():
+        """Test normal operation when all arguments and options are specified"""
+
+        TestResultsPush.add_http_mock_response(
+            code=HTTPStatus.REQUEST_ENTITY_TOO_LARGE, message="Request Entity Too Large"
+        )
+        runner = CliRunner(mix_stderr=False)
+        result = runner.invoke(
+            main,
+            args=[
+                TestResultsPush.TOKN_SWITCH,
+                TestResultsPush.TOKN_TEXT,
+                TestResultsPush.ACCESS_SWITCH,
+                TestResultsPush.ACCESS_TEXT,
+                TestResultsPush.META_SWITCH,
+                TestResultsPush.META_TEXT_TEST + "," + TestResultsPush.META_TEXT_FOO,
+                tarball,
+            ],
+        )
+        assert result.exit_code == 1, result.stderr
+        assert (
+            result.stderr
+            == "HTTP Error status: 413, message: Request Entity Too Large\n"
+        )
 
     @staticmethod
     @responses.activate
@@ -196,7 +246,7 @@ def test_token_envar(monkeypatch, caplog):
         runner = CliRunner(mix_stderr=False)
         result = runner.invoke(main, args=[tarball])
         assert result.exit_code == 0, result.stderr
-        assert result.stderr == "File uploaded successfully\n"
+        assert result.stdout == ""
 
     @staticmethod
     @responses.activate
@@ -239,7 +289,9 @@ def test_http_error(monkeypatch, caplog):
         """Test handling of 404 errors"""
 
         monkeypatch.setenv("PBENCH_ACCESS_TOKEN", TestResultsPush.TOKN_TEXT)
-        TestResultsPush.add_http_mock_response(HTTPStatus.NOT_FOUND)
+        TestResultsPush.add_http_mock_response(
+            HTTPStatus.NOT_FOUND, message="Not Found"
+        )
         caplog.set_level(logging.DEBUG)
         runner = CliRunner(mix_stderr=False)
         result = runner.invoke(main, args=[tarball])