From f30041b2e6393f9f249cb0bec0f64158ed5fb27c Mon Sep 17 00:00:00 2001
From: Viacheslav Kukushkin <vy.kukushkin@gmail.com>
Date: Sun, 4 Feb 2024 22:56:53 +0300
Subject: [PATCH] Added testing on debug-logging env for docker containers

---
 cli/cli_tests.sh                              | 41 ++++++++++++++++---
 .../commands/compatibility_test/run.py        |  5 ++-
 cli/medperf/entities/cube.py                  | 11 +++--
 cli/medperf/entities/report.py                |  2 +-
 cli/medperf/utils.py                          | 11 ++---
 cli/tests_setup.sh                            |  7 +++-
 6 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/cli/cli_tests.sh b/cli/cli_tests.sh
index df803391f..2b8da2cd5 100755
--- a/cli/cli_tests.sh
+++ b/cli/cli_tests.sh
@@ -97,28 +97,36 @@ echo "====================================="
 
 medperf mlcube submit --name prep -m $PREP_MLCUBE -p $PREP_PARAMS
 checkFailed "Prep submission failed"
-PREP_UID=$(medperf mlcube ls | tail -n 1 | tr -s ' ' | cut -d ' ' -f 2)
+PREP_UID=$(medperf mlcube ls | grep prep | head -n 1 | tr -s ' ' | cut -d ' ' -f 2)
 
 medperf mlcube submit --name model1 -m $MODEL_MLCUBE -p $MODEL1_PARAMS -a $MODEL_ADD
 checkFailed "Model1 submission failed"
-MODEL1_UID=$(medperf mlcube ls | tail -n 1 | tr -s ' ' | cut -d ' ' -f 2)
+MODEL1_UID=$(medperf mlcube ls | grep model1 | head -n 1 | tr -s ' ' | cut -d ' ' -f 2)
 
 medperf mlcube submit --name model2 -m $MODEL_MLCUBE -p $MODEL2_PARAMS -a $MODEL_ADD
 checkFailed "Model2 submission failed"
-MODEL2_UID=$(medperf mlcube ls | tail -n 1 | tr -s ' ' | cut -d ' ' -f 2)
+MODEL2_UID=$(medperf mlcube ls | grep model2 | head -n 1 | tr -s ' ' | cut -d ' ' -f 2)
 
 # MLCube with singularity section
 medperf mlcube submit --name model3 -m $MODEL_WITH_SINGULARITY -p $MODEL3_PARAMS -a $MODEL_ADD -i $MODEL_SING_IMAGE
 checkFailed "Model3 submission failed"
-MODEL3_UID=$(medperf mlcube ls | tail -n 1 | tr -s ' ' | cut -d ' ' -f 2)
+MODEL3_UID=$(medperf mlcube ls | grep model3 | head -n 1 | tr -s ' ' | cut -d ' ' -f 2)
 
 medperf mlcube submit --name model-fail -m $FAILING_MODEL_MLCUBE -p $MODEL4_PARAMS -a $MODEL_ADD
 checkFailed "failing model submission failed"
-FAILING_MODEL_UID=$(medperf mlcube ls | tail -n 1 | tr -s ' ' | cut -d ' ' -f 2)
+FAILING_MODEL_UID=$(medperf mlcube ls | grep model-fail | head -n 1 | tr -s ' ' | cut -d ' ' -f 2)
+
+medperf mlcube submit --name model-log-none -m MODEL_LOG_MLCUBE -p $MODEL_LOG_NONE_PARAMS
+checkFailed "Model with logging None submission failed"
+MODEL_LOG_NONE_UID=$(medperf mlcube ls | grep model-fail | head -n 1 | tr -s ' ' | cut -d ' ' -f 2)
+
+medperf mlcube submit --name model-log-debug -m MODEL_LOG_MLCUBE -p $MODEL_LOG_DEBUG_PARAMS
+checkFailed "Model with logging debug submission failed"
+MODEL_LOG_DEBUG_UID=$(medperf mlcube ls | grep model-fail | head -n 1 | tr -s ' ' | cut -d ' ' -f 2)
 
 medperf mlcube submit --name metrics -m $METRIC_MLCUBE -p $METRIC_PARAMS
 checkFailed "Metrics submission failed"
-METRICS_UID=$(medperf mlcube ls | tail -n 1 | tr -s ' ' | cut -d ' ' -f 2)
+METRICS_UID=$(medperf mlcube ls | grep model-fail | head -n 1 | tr -s ' ' | cut -d ' ' -f 2)
 ##########################################################
 
 echo "\n"
@@ -356,6 +364,27 @@ checkFailed "Failing mlcube run with ignore errors failed"
 
 echo "\n"
 
+##########################################################
+echo "====================================="
+echo "Running logging model without logging env"
+echo "====================================="
+medperf benchmark run -b $BMK_UID -d $DSET_A_UID -m $MODEL_LOG_NONE_UID
+checkFailed "run logging model without logging env failed"
+##########################################################
+
+echo "\n"
+
+##########################################################
+echo "====================================="
+echo "Running logging model with debug logging env"
+echo "====================================="
+# TODO: this MUST fail for now
+medperf benchmark run -b $BMK_UID -d $DSET_A_UID -m $MODEL_LOG_DEBUG_UID
+checkFailed "run logging model with debug logging env failed"
+##########################################################
+
+echo "\n"
+
 ##########################################################
 echo "====================================="
 echo "Logout users"
diff --git a/cli/medperf/commands/compatibility_test/run.py b/cli/medperf/commands/compatibility_test/run.py
index 0127633f2..15f2de04e 100644
--- a/cli/medperf/commands/compatibility_test/run.py
+++ b/cli/medperf/commands/compatibility_test/run.py
@@ -1,5 +1,4 @@
 import logging
-from typing import List
 
 from medperf.commands.execution import Execution
 from medperf.entities.dataset import Dataset
@@ -26,7 +25,7 @@ def run(
         data_uid: str = None,
         no_cache: bool = False,
         offline: bool = False,
-    ) -> List:
+    ) -> (str, dict):
         """Execute a test workflow. Components of a complete workflow should be passed.
         When only the benchmark is provided, it implies the following workflow will be used:
         - the benchmark's demo dataset is used as the raw data
@@ -89,6 +88,8 @@ def run(
         test_exec.prepare_dataset()
         test_exec.initialize_report()
         results = test_exec.cached_results()
+        logging.info('Existing results are found. Test would not be re-executed.')
+        logging.debug(f'Existing results: {results}')
         if results is None:
             results = test_exec.execute()
             test_exec.write(results)
diff --git a/cli/medperf/entities/cube.py b/cli/medperf/entities/cube.py
index 48beddd63..5b0c66270 100644
--- a/cli/medperf/entities/cube.py
+++ b/cli/medperf/entities/cube.py
@@ -131,6 +131,7 @@ def __local_all(cls) -> List["Cube"]:
         cubes_folder = config.cubes_folder
         try:
             uids = next(os.walk(cubes_folder))[1]
+            logging.debug(f'Local cubes found: {uids}')
         except StopIteration:
             msg = "Couldn't iterate over cubes directory"
             logging.warning(msg)
@@ -322,6 +323,8 @@ def run(
             cmd_arg = f'{k}="{v}"'
             cmd = " ".join([cmd, cmd_arg])
 
+        container_loglevel = config.container_loglevel
+
         # TODO: we should override run args instead of what we are doing below
         #       we shouldn't allow arbitrary run args unless our client allows it
         if config.platform == "docker":
@@ -332,6 +335,9 @@ def run(
             gpu_args = " ".join([gpu_args, "-u $(id -u):$(id -g)"]).strip()
             cmd += f' -Pdocker.cpu_args="{cpu_args}"'
             cmd += f' -Pdocker.gpu_args="{gpu_args}"'
+
+            if container_loglevel:
+                cmd += f' -Pdocker.env_args="-e MEDPERF_LOGLEVEL={container_loglevel}"'
         elif config.platform == "singularity":
             # use -e to discard host env vars, -C to isolate the container (see singularity run --help)
             run_args = self.get_config("singularity.run_args") or ""
@@ -346,6 +352,7 @@ def run(
                 cmd += (
                     f' -Psingularity.image="{self._converted_singularity_image_name}"'
                 )
+            # TODO: pass logging env for singularity also there
         else:
             raise InvalidArgumentError("Unsupported platform")
 
@@ -353,10 +360,6 @@ def run(
         # force mlcube to only use --gpus to figure out GPU config
         cmd += " -Pplatform.accelerator_count=0"
 
-        container_loglevel = config.container_loglevel
-        if container_loglevel:
-            cmd += f' -Pdocker.env_args="-e LOGLEVEL={container_loglevel}"'
-
         logging.info(f"Running MLCube command: {cmd}")
         proc = pexpect.spawn(cmd, timeout=timeout)
         proc_out = combine_proc_sp_text(proc)
diff --git a/cli/medperf/entities/report.py b/cli/medperf/entities/report.py
index 54621280d..a0069d847 100644
--- a/cli/medperf/entities/report.py
+++ b/cli/medperf/entities/report.py
@@ -14,7 +14,7 @@ class TestReport(Entity, MedperfBaseSchema):
     """
     Class representing a compatibility test report entry
 
-    A test report is comprised of the components of a test execution:
+    A test report consists of the components of a test execution:
     - data used, which can be:
         - a demo dataset url and its hash, or
         - a raw data path and its labels path, or
diff --git a/cli/medperf/utils.py b/cli/medperf/utils.py
index ec56d1572..528806ca9 100644
--- a/cli/medperf/utils.py
+++ b/cli/medperf/utils.py
@@ -199,6 +199,7 @@ def dict_pretty_print(in_dict: dict, skip_none_values: bool = True):
 
     Args:
         in_dict (dict): dictionary to print
+        skip_none_values (bool): if fields with `None` values should be omitted
     """
     logging.debug(f"Printing dictionary to the user: {in_dict}")
     ui = config.ui
@@ -219,7 +220,6 @@ def combine_proc_sp_text(proc: spawn) -> str:
 
     Args:
         proc (spawn): a pexpect spawned child
-        ui (UI): An instance of an UI implementation
 
     Returns:
         str: all non-carriage-return-ending string captured from proc
@@ -247,13 +247,13 @@ def combine_proc_sp_text(proc: spawn) -> str:
 
 def get_folders_hash(paths: List[str]) -> str:
     """Generates a hash for all the contents of the fiven folders. This procedure
-    hashes all of the files in all passed folders, sorts them and then hashes that list.
+    hashes all the files in all passed folders, sorts them and then hashes that list.
 
     Args:
-        path (str): Folder to hash
+        paths List(str): Folders to hash.
 
     Returns:
-        str: sha256 hash of the whole folder
+        str: sha256 hash that represents all the folders altogether
     """
     hashes = []
 
@@ -278,7 +278,7 @@ def list_files(startpath):
     tree_str = ""
     for root, dirs, files in os.walk(startpath):
         level = root.replace(startpath, "").count(os.sep)
-        indent = " " * 4 * (level)
+        indent = " " * 4 * level
 
         tree_str += "{}{}/\n".format(indent, os.path.basename(root))
         subindent = " " * 4 * (level + 1)
@@ -353,6 +353,7 @@ def get_cube_image_name(cube_path: str) -> str:
         cube_config = yaml.safe_load(f)
 
     try:
+        # TODO: Why do we check singularity only there? Why not docker?
         return cube_config["singularity"]["image"]
     except KeyError:
         msg = "The provided mlcube doesn't seem to be configured for singularity"
diff --git a/cli/tests_setup.sh b/cli/tests_setup.sh
index 5f9c376a1..05f80beeb 100644
--- a/cli/tests_setup.sh
+++ b/cli/tests_setup.sh
@@ -61,7 +61,8 @@ fi
 ##########################################################
 ########################## Setup #########################
 ##########################################################
-ASSETS_URL="https://raw.githubusercontent.com/hasan7n/mockcube/32294ec0babbcb7773e40075e605ed6f18f6b125"
+# TODO: return back to hasan7n's repo after logging model is merged there
+ASSETS_URL="https://raw.githubusercontent.com/VukW/mockcube/07de6f0ef8cc422344ee3496cd6131e7b010294b"
 
 # datasets
 DSET_A_URL="$ASSETS_URL/assets/datasets/dataset_a.tar.gz"
@@ -77,6 +78,7 @@ PREP_PARAMS="$ASSETS_URL/prep-sep/mlcube/workspace/parameters.yaml"
 FAILING_MODEL_MLCUBE="$ASSETS_URL/model-bug/mlcube/mlcube.yaml" # doesn't fail with association
 MODEL_WITH_SINGULARITY="$ASSETS_URL/model-cpu/mlcube/mlcube_docker+singularity.yaml"
 MODEL_MLCUBE="$ASSETS_URL/model-cpu/mlcube/mlcube.yaml"
+MODEL_LOG_MLCUBE="$ASSETS_URL/model-debug-logging/mlcube/mlcube.yaml"
 MODEL_ADD="$ASSETS_URL/assets/weights/weights1.tar.gz"
 MODEL_SING_IMAGE="$ASSETS_URL/model-cpu/mlcube/workspace/.image/mock-model-cpu.simg"
 
@@ -85,6 +87,9 @@ MODEL2_PARAMS="$ASSETS_URL/model-cpu/mlcube/workspace/parameters2.yaml"
 MODEL3_PARAMS="$ASSETS_URL/model-cpu/mlcube/workspace/parameters3.yaml"
 MODEL4_PARAMS="$ASSETS_URL/model-cpu/mlcube/workspace/parameters4.yaml"
 
+MODEL_LOG_NONE_PARAMS="$ASSETS_URL/model-debug-logging/mlcube/workspace/parameters_none.yaml"
+MODEL_LOG_DEBUG_PARAMS="$ASSETS_URL/model-debug-logging/mlcube/workspace/parameters_debug.yaml"
+
 # metrics cubes
 METRIC_MLCUBE="$ASSETS_URL/metrics/mlcube/mlcube.yaml"
 METRIC_PARAMS="$ASSETS_URL/metrics/mlcube/workspace/parameters.yaml"