Address review comments and various other problems

- Change copy_result_tb return the response without interpretation.

- Change push.py to interpret the returned response and act accordingly.
There is still output in the "normal" case, mainly to indicate a newly
created upload vs a duplicate.

- move.py gets the response but does not do anything with it yet.

lib/pbench/agent/results.py

@@ -279,12 +279,12 @@ def make_result_tb(self, single_threaded: bool = False) -> TarballRecord:
 
 
 class ResponseStatus:
-
     def __init__(self, status_code: int, reason: str, message: str):
         self.status_code = status_code
         self.reason = reason
         self.message = message
 
+
 class CopyResultTb:
     """Interfaces for copying result tar balls remotely using the server's HTTP
     PUT method for uploads.
@@ -320,7 +320,7 @@ def __init__(
 
     def copy_result_tb(
         self, token: str, access: Optional[str] = None, metadata: Optional[List] = None
-    ) -> ResponseStatus:
+    ) -> requests.models.Response:
         """Copies the tar ball from the agent to the configured server using upload
         API.
 
@@ -375,10 +375,7 @@ def copy_result_tb(
                 )
 
                 response = requests.Session().send(request)
-                return ResponseStatus(response.status_code,
-                                      response.reason,
-                                      response.text
-                                      )
+                return response
             except requests.exceptions.ConnectionError as exc:
                 raise RuntimeError(f"Cannot connect to '{self.upload_url}': {exc}")
             except Exception as exc:

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
@@ -27,15 +28,24 @@ def execute(self) -> int:
         res = crt.copy_result_tb(
             self.context.token, self.context.access, self.context.metadata
         )
-        if 200 <= res.status_code <= 201:
-            click.echo(f"Status: {res.status_code}, reason: {res.reason}, message: {res.message}")
+
+        try:
+            msg = res.json()["message"]
+        except requests.exceptions.JSONDecodeError:
+            msg = "FOO"
+
+        if res.ok:
+            click.echo(
+                f"Status: {res.status_code}, reason: {res.reason}, message: {msg}"
+            )
             return 0
-        elif res.status_code >= 400:
-            click.echo(f"Error status: {res.status_code}, reason: {res.reason}, message: {res.message}", err = True)
-            return 1
         else:
-            click.echo(f"Unexpected status: {res.status_code}, reason: {res.reason}, message: {res.message}", err = True)
-            return 2
+            click.echo(
+                f"Error status: {res.status_code}, reason: {res.reason}, message: {msg}",
+                err=True,
+            )
+            return 1
+
 
 @sort_click_command_parameters
 @click.command(name="pbench-results-push")

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

@@ -148,7 +148,7 @@ def test_multiple_meta_args_single_option():
             ],
         )
         assert result.exit_code == 0, result.stderr
-        assert result.stdout == "Status: 200, reason: OK, message: \n"
+        assert result.stdout == "Status: 200, reason: OK, message: FOO\n"
 
     @staticmethod
     @responses.activate
@@ -172,7 +172,7 @@ def test_multiple_meta_args():
             ],
         )
         assert result.exit_code == 0, result.stderr
-        assert result.stdout == "Status: 200, reason: OK, message: \n"
+        assert result.stdout == "Status: 200, reason: OK, message: FOO\n"
 
     @staticmethod
     @responses.activate
@@ -194,7 +194,7 @@ def test_normal_created():
             ],
         )
         assert result.exit_code == 0, result.stderr
-        assert result.stdout == "Status: 201, reason: Created, message: \n"
+        assert result.stdout == "Status: 201, reason: Created, message: FOO\n"
 
     @staticmethod
     @responses.activate
@@ -216,29 +216,10 @@ def test_error_too_large_tarball():
             ],
         )
         assert result.exit_code == 1, result.stderr
-        assert result.stderr == "Error status: 413, reason: Request Entity Too Large, message: \n"
-
-    @staticmethod
-    @responses.activate
-    def test_unexpected_status():
-        """Test normal operation when all arguments and options are specified"""
-
-        TestResultsPush.add_http_mock_response(code=HTTPStatus.SEE_OTHER)
-        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.stderr
+            == "Error status: 413, reason: Request Entity Too Large, message: FOO\n"
         )
-        assert result.exit_code == 2, result.stderr
-        assert result.stderr == "Unexpected status: 303, reason: See Other, message: \n"
 
     @staticmethod
     @responses.activate
@@ -262,7 +243,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.stdout == "Status: 200, reason: OK, message: \n"
+        assert result.stdout == "Status: 200, reason: OK, message: FOO\n"
 
     @staticmethod
     @responses.activate