Remove files list from status info; update README.md (#22)

Also, enhance the response from the server status API with additional
value fields.

PBENCH-1258

Author: webbnh

README.md

@@ -33,11 +33,8 @@ This utility currently offers five methods:
 - `GET /<server_id>`: return server status
 - `DELETE /<server_id>`: request server shutdown
 
-There are a number of tweaks which are expected to be added:
-- The hash algorithm for resource names may be changed or made configurable
-- The underlying web server may be changed from the reference one to Gunicorn or other
-- The web server should be made able to accept SSL connections
-- The utility needs to be packaged, either as a Python package or a container (or both)
-- Figure out what the server status response _should_ contain -- currently, it provides
-a list of the available files, which undermines the "ya gotta know it's there" story.
-
+There are a number of tweaks which should be considered:
+- Change the hash algorithm for resource names or make it configurable
+- Change the underlying web server from the reference one to Gunicorn or other
+- Make the web server able to accept SSL connections or place it behind a
+suitably-configured proxy inside the container.

src/relay/relay.py

@@ -6,7 +6,6 @@
 import os
 from pathlib import Path
 import shutil
-import subprocess
 from typing import Callable
 
 from bottle import Bottle, HTTPResponse, request, static_file
@@ -30,12 +29,16 @@
 app = Bottle()
 
 
-def get_disk_utilization_str(dir_path: Path) -> str:
+def get_disk_utilization(dir_path: Path) -> dict[str, int | str]:
     usage = shutil.disk_usage(dir_path)
-    return "{:.3}% full, {} remaining".format(
-        float(usage.used) / float(usage.total) * 100.0,
-        humanize.naturalsize(usage.free),
-    )
+    return {
+        "bytes_used": usage.used,
+        "bytes_remaining": usage.free,
+        "string": "{:.3}% full, {} remaining".format(
+            float(usage.used) / float(usage.total) * 100.0,
+            humanize.naturalsize(usage.free),
+        ),
+    }
 
 
 def validate_server_id(func: Callable) -> Callable:
@@ -103,20 +106,10 @@ def relay_status(context: click.Context) -> HTTPResponse:
 
     Returns:
         An HTTP response with a status of OK and a JSON payload containing
-        status information (currently, the output from `ls` listing the files
-        in the upload directory).
+        status information.
     """
     logging.info("request to report status")
-    body = {"disk utilization": get_disk_utilization_str(context.meta[CTX_DIRECTORY])}
-
-    cp = subprocess.run(
-        ["ls", "-l"], cwd=context.meta[CTX_DIRECTORY], capture_output=True, text=True
-    )
-    if cp.returncode:
-        body["error"] = cp.stderr.strip()
-    else:
-        body["files"] = cp.stdout.strip().split("\n")
-
+    body = {"disk utilization": get_disk_utilization(context.meta[CTX_DIRECTORY])}
     return HTTPResponse(status=HTTPStatus.OK, body=body)
 
 
@@ -182,7 +175,7 @@ def receive_file(context: click.Context, file_id: str) -> HTTPResponse:
     logging.info(
         'request to upload file id "%s", disk %s',
         file_id,
-        get_disk_utilization_str(context.meta[CTX_DIRECTORY]),
+        get_disk_utilization(context.meta[CTX_DIRECTORY])["string"],
     )
 
     if not 0 < request.content_length <= FILE_MAX_SIZE:
@@ -255,7 +248,7 @@ def receive_file(context: click.Context, file_id: str) -> HTTPResponse:
             logging.info(
                 'file id "%s" uploaded successfully, disk %s',
                 file_id,
-                get_disk_utilization_str(context.meta[CTX_DIRECTORY]),
+                get_disk_utilization(context.meta[CTX_DIRECTORY])["string"],
             )
         else:
             logging.info(

tests/test_relay.py

@@ -5,8 +5,6 @@
 import os
 from pathlib import Path
 import shutil
-import subprocess
-from subprocess import CompletedProcess
 from typing import Any, Callable, IO, NamedTuple, Optional, Union
 
 from _pytest.monkeypatch import MonkeyPatch
@@ -98,7 +96,12 @@ class TestRelay:
     FILDIR_SWITCH = "--files-directory"
     BDEBUG_SWITCH = "--debug"
 
-    DISK_STR = "We've got disk!"
+    FAKE_DISK_UTL = {
+        "bytes_used": 1200000,
+        "bytes_remaining": 80000000000,
+        "string": "0.00001% full, 80Gb remaining",
+    }
+
     END_OF_MAIN = AssertionError("Unexpectedly reached the body of main()")
     SERVER_ID_TEXT = "ThisIsMyServerServerID"
 
@@ -170,10 +173,10 @@ def do_setup(
           - pathlib.Path.open()
           - pathlib.Path.unlink()
           - the Path "/" operators
-          - relay.get_disk_utilization_str()
+          - relay.get_disk_utilization()
 
         The mock classes seem to be necessary in order to intercept the
-        respective member functions, possibly because these native
+        respective member functions, possibly because these are native
         implementations instead of "pure Python" (or, maybe I just don't
         know what I'm doing).
 
@@ -331,19 +334,19 @@ def unlink(self, *args, **kwargs):
                 else:
                     return self.path.unlink(*args, **kwargs)
 
-        def mock_get_disk_utilization_str(dir_path: Path) -> str:
-            """Mock for relay.get_disk_utilization_str()
+        def mock_get_disk_utilization(dir_path: Path) -> dict[str, int | str]:
+            """Mock for relay.get_disk_utilization()
 
             Returns a static string.
 
             Note that if the assertion fails, the exception will be caught and
             reported as an INTERNAL_SERVER_ERROR.  This will likely make the
-            test fail, but only if it's checking the response....
+            test fail, but only if it's actually checking the response....
             """
             assert str(dir_path) == relay.DEFAULT_FILES_DIRECTORY
-            return TestRelay.DISK_STR
+            return TestRelay.FAKE_DISK_UTL
 
-        m.setattr(relay, "get_disk_utilization_str", mock_get_disk_utilization_str)
+        m.setattr(relay, "get_disk_utilization", mock_get_disk_utilization)
         m.setattr(relay, "Path", MockPath)
         m.setattr(relay, "sha256", lambda *_args, **_kwargs: MockHash())
         m.setattr(relay.app, "run", func)
@@ -490,28 +493,13 @@ def test_command_with_server_error(monkeypatch: MonkeyPatch):
         TestRelay.check_result(result, exit_code=2, stderr="Error running the server")
 
     @staticmethod
-    @pytest.mark.parametrize(
-        "files_str,returncode",
-        (("We've got files!", 0), ("We've got NO files!", 1)),
-    )
-    def test_relay_status_operation(
-        files_str: str, returncode: int, monkeypatch: MonkeyPatch
-    ):
+    def test_relay_status_operation(monkeypatch: MonkeyPatch):
         """Test GET /<server_id> method operation"""
 
-        def mock_run(args: Union[str, list[str]], *, cwd: str, **_kwargs):
-            """Mock for subprocess.run()"""
-            assert str(cwd) == relay.DEFAULT_FILES_DIRECTORY
-            key = "stdout" if returncode == 0 else "stderr"
-            kwargs = {"args": args, "returncode": returncode, key: files_str}
-            return CompletedProcess(**kwargs)
-
         def validate_relay(response: HTTPResponse):
             """Validate the response from the HTTP method call"""
             assert response.status_code == HTTPStatus.OK
-            assert TestRelay.DISK_STR in response.body["disk utilization"]
-            key = "files" if returncode == 0 else "error"
-            assert files_str in response.body[key]
+            assert response.body["disk utilization"] == TestRelay.FAKE_DISK_UTL
 
         with monkeypatch.context() as m:
             mock = mock_app_method_call(
@@ -520,7 +508,6 @@ def validate_relay(response: HTTPResponse):
                 method_args={"server_id": TestRelay.SERVER_ID_TEXT},
             )
             TestRelay.do_setup(m, func=mock)
-            m.setattr(subprocess, "run", mock_run)
             result = TestRelay.invoke_main()
         TestRelay.check_result(result)
 
@@ -853,8 +840,8 @@ def validate_receive_file(response: HTTPResponse):
         assert request.content_length == bytes_written[0]
 
     @staticmethod
-    def test_get_disk_utilization_str(monkeypatch: MonkeyPatch):
-        """Exercise get_disk_utilization_str()
+    def test_get_disk_utilization(monkeypatch: MonkeyPatch):
+        """Exercise get_disk_utilization()
 
         This is a (nearly) trivial function, but we test it so that the unit
         tests show 100% coverage of the CUT.
@@ -867,6 +854,7 @@ class DiskUsageData(NamedTuple):
 
         expected_dir_path = Path("/mockdir")
         du = DiskUsageData()
+        nv = "3.2 GB"
 
         def mock_disk_usage(dir_path: Path) -> DiskUsageData:
             """Mock shutil.disk_usage()"""
@@ -877,13 +865,20 @@ def mock_naturalsize(value: Union[float, str], *args):
             """Mock humanize.naturalsize()"""
             assert len(args) == 0
             assert str(value) == str(du.free)
-            return "3.2 GB"
+            return nv
 
         with monkeypatch.context() as m:
             m.setattr(shutil, "disk_usage", mock_disk_usage)
             m.setattr(humanize, "naturalsize", mock_naturalsize)
-            expected = "40.0% full, 3.2 GB remaining"
-            actual = relay.get_disk_utilization_str(expected_dir_path)
+            expected = {
+                "bytes_used": du.used,
+                "bytes_remaining": du.free,
+                "string": "{:.3}% full, {} remaining".format(
+                    float(du.used) / float(du.total) * 100.0,
+                    nv,
+                ),
+            }
+            actual = relay.get_disk_utilization(expected_dir_path)
             assert actual == expected
 
     @staticmethod