fix: handle incomplete JS strings

.github/workflows/coverage.yml

@@ -8,6 +8,6 @@ jobs:
     - uses: wntrblm/nox@2022.11.21
       with:
         python-versions: "3.9, 3.10, 3.11"
-    - run: pipx install poetry==1.3.1
-    - run: pipx inject poetry poetry-plugin-export
+    - run: pipx install poetry==2.2.1
+    - run: poetry self add poetry-plugin-export
     - run: nox --sessions tests-3.10 coverage

.github/workflows/release.yml

@@ -10,8 +10,8 @@ jobs:
     - uses: wntrblm/nox@2022.11.21
       with:
         python-versions: "3.9, 3.10, 3.11"
-    - run: pipx install poetry==1.3.1
-    - run: pipx inject poetry poetry-plugin-export
+    - run: pipx install poetry==2.2.1
+    - run: poetry self add poetry-plugin-export
     - run: nox
     - run: poetry build
     - run: poetry publish --username=__token__ --password=${{ secrets.PYPI_TOKEN }}

.github/workflows/tests.yml

@@ -8,6 +8,6 @@ jobs:
     - uses: wntrblm/nox@2022.11.21
       with:
         python-versions: "3.9, 3.10, 3.11"
-    - run: pipx install poetry==1.3.1
-    - run: pipx inject poetry poetry-plugin-export
+    - run: pipx install poetry==2.2.1
+    - run: poetry self add poetry-plugin-export
     - run: nox

noxfile.py

@@ -79,6 +79,7 @@ def lint(session):
 def pip_audit(session):
     """Scan dependencies for insecure packages."""
     with tempfile.NamedTemporaryFile() as requirements:
+        session.run("python", "-m", "pip", "install", "--upgrade", "pip")
         session.run(
             "poetry",
             "export",
@@ -92,8 +93,6 @@ def pip_audit(session):
         install_with_constraints(session, "pip-audit")
         session.run(
             "pip-audit",
-            "-r",
-            requirements.name,
         )
 
 

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "reportseff"
-version = "2.10.0"
+version = "2.10.1"
 description= "Tablular seff output"
 authors = ["Troy Comi <troycomi@gmail.com>"]
 license = "MIT"
@@ -33,6 +33,9 @@ reportseff = "reportseff.console:main"
 [tool.poetry.group.dev-dependencies.dependencies]
 pip-audit = "^2.9.0"
 
+[tool.poetry.requires-plugins]
+poetry-plugin-export = ">=1.8"
+
 [build-system]
 requires = ["poetry-core>=1.0.0"]
 build-backend = "poetry.core.masonry.api"

src/reportseff/job.py

@@ -217,8 +217,12 @@ def _parse_admin_comment(self, comment: str) -> None:
         for node, node_data in data["nodes"].items():
             self.comment_data[node] = _get_node_data(data, node_data)
 
-        self.cpu = _average_nested_dict("CPUEff", self.comment_data)
-        self.mem_eff = _average_nested_dict("MemEff", self.comment_data)
+        cpu_eff = _average_nested_dict("CPUEff", self.comment_data)
+        if cpu_eff is not None:
+            self.cpu = cpu_eff
+        mem_eff = _average_nested_dict("MemEff", self.comment_data)
+        if mem_eff is not None:
+            self.mem_eff = mem_eff
         if data["gpus"]:
             self.gpu = _average_nested_dict("GPUEff", self.comment_data)
             self.gpu_mem = _average_nested_dict("GPUMem", self.comment_data)
@@ -490,12 +494,12 @@ def get_gpu_value(comment_data: dict, key: str, gpu_number: int) -> float:
             return comment_data[key][gpu_number]
         return 0
 
-    result = {
-        "MemEff": node_data["used_memory"] / node_data["total_memory"] * 100,
-    }
+    result = {}
+    if "used_memory" in node_data:
+        result["MemEff"] = node_data["used_memory"] / node_data["total_memory"] * 100
 
-    if node_data["cpus"] == 0 or comment_data["total_time"] == 0:
-        result["CPUEff"] = 0
+    if node_data.get("cpus", 0) == 0 or comment_data.get("total_time", 0) == 0:
+        pass
     else:
         time_per_cpu = node_data["total_time"] / node_data["cpus"]
         result["CPUEff"] = time_per_cpu / comment_data["total_time"] * 100
@@ -518,7 +522,7 @@ def get_gpu_value(comment_data: dict, key: str, gpu_number: int) -> float:
     return result
 
 
-def _average_nested_dict(nested_key: str, data: dict) -> float:
+def _average_nested_dict(nested_key: str, data: dict) -> float | None:
     """Average nested values in data dictionary.
 
     Args:
@@ -528,8 +532,7 @@ def _average_nested_dict(nested_key: str, data: dict) -> float:
     Returns:
         the mean value, rounded to one decimal
     """
-    return round(
-        sum(value[nested_key] for value in data.values() if nested_key in value)
-        / len(data),
-        1,
-    )
+    values = [value[nested_key] for value in data.values() if nested_key in value]
+    if values == []:
+        return None
+    return round(sum(values) / len(values), 1)

tests/conftest.py

@@ -1,5 +1,7 @@
 """Collect common fixtures."""
 
+from __future__ import annotations
+
 import base64
 import gzip
 import json
@@ -20,6 +22,24 @@ def get_jobstats():
     return to_comment
 
 
+@pytest.fixture()
+def strip_js():
+    """Removes entries from jobstats string, converting to dict internally."""
+
+    # wrap to make a fixture
+    def strip_js_inner(js_string: str, to_remove: list[str]):
+        info = json.loads(gzip.decompress(base64.b64decode(js_string[4:])))
+        for token in to_remove:
+            if token in info:
+                info.pop(token)
+            for value in info["nodes"].values():
+                if token in value:
+                    value.pop(token)
+        return to_comment(info)
+
+    return strip_js_inner
+
+
 def to_sacct_dict(sacct_line: str) -> dict:
     """Convert debug print statement to dictionary like from sacct."""
     columns = (

tests/test_job.py

@@ -770,7 +770,7 @@ def test_issue_26(get_jobstats):
     job = job_module.Job("13421658", "13421658", None)
     job.update(entry)
     assert job.state == "FAILED"
-    assert job.cpu == 0
+    assert job.cpu is None
     assert job.mem_eff == 0
 
 

tests/test_reportseff.py

@@ -1096,3 +1096,161 @@ def test_issue_73_totals(mocker):
         "COMPLETED",
     ]
     assert len(output) == 1
+
+
+@pytest.mark.usefixtures("_mock_inquirer")
+def test_issue_84_empty_used_memory(mocker):
+    """Crash when the used memory in JS entry is empty."""
+    mocker.patch("reportseff.console.which", return_value=True)
+    runner = CliRunner()
+    sub_result = mocker.MagicMock()
+    sub_result.returncode = 0
+    js_string = (
+        "JS1:H4sIAJnmcGkC/13OTQ4CIQwF4LuwVtKiQ8hcZoJSDQk/hikLZ8LdRdSN274v73UX"
+        "KTtaxbyLO4CS11rYyo2cZXmrxwuV4JN0NMCjLpzZhiVSzOX5vikxIxpE1AD63A4D1ZX"
+        "cn4HpZNAY8xPsg98s+5y+Alrr2aeefeyDatID9+ewvQBAtRncqAAAAA=="
+    )
+    base_output = (
+        "^|^1^|^00:01:17^|^12345678^|^12345678^|^^|^1^|^^|^4000M^|^COMPLETED^|^01:00:00^|^00:02.833^|^\n"
+        "^|^1^|^00:01:17^|^12345678.batch^|^12345678.batch^|^138120K^|^1^|^1^|^^|^COMPLETED^|^^|^00:02.833^|^\n"
+        "^|^1^|^00:01:17^|^12345678.extern^|^12345678.extern^|^^|^1^|^1^|^^|^COMPLETED^|^^|^00:00:00^|^\n"
+        "^|^1^|^00:00:43^|^12345678.0^|^12345678.0^|^2200M^|^1^|^1^|^^|^COMPLETED^|^^|^00:00:00^|^\n"
+    )
+    sub_result.stdout = f"{js_string}{base_output}"
+    mocker.patch("reportseff.db_inquirer.subprocess.run", return_value=sub_result)
+    result = runner.invoke(
+        console.main,
+        [
+            "--no-color",
+            "12345678",
+        ],
+    )
+
+    assert result.exit_code == 0
+    # remove header
+    output = result.output.split("\n")[1:-1]
+    assert output[0].split() == [
+        "12345678",
+        "COMPLETED",
+        "00:01:17",
+        "2.1%",
+        "2.6%",
+        "55.0%",
+    ]
+    assert len(output) == 1
+
+
+@pytest.mark.usefixtures("_mock_inquirer")
+def test_nonempty_used_memory(mocker, strip_js):
+    """Check admin comment is used when available."""
+    mocker.patch("reportseff.console.which", return_value=True)
+    runner = CliRunner()
+    sub_result = mocker.MagicMock()
+    sub_result.returncode = 0
+    js_string = (
+        "JS1:H4sIANsKfWkC/13MQQqDMBCF4bvMOi0zmREaLyOSDFJITNG4KJK7m1pw4fb/eG+"
+        "HOQddod8haIzjY2EvM8kvlFzGOCRNeflCL+SEURDRwLZquIAcd8iWbYP/pLyTto4v97"
+        "QG/Gdr/1TrjYmdgelErAceV8ooiAAAAA=="
+    )
+    base_output = (
+        "^|^1^|^00:18:59^|^4336165^|^4336165^|^^|^1^|^^|^4000M^|^COMPLETED^|^01:30:00^|^18:41.934^|^\n"
+        "^|^1^|^00:18:59^|^4336165.batch^|^4336165.batch^|^4094320K^|^1^|^1^|^^|^COMPLETED^|^^|^18:41.934^|^\n"
+        "^|^1^|^00:18:59^|^4336165.extern^|^4336165.extern^|^^|^1^|^1^|^^|^COMPLETED^|^^|^00:00:00^|^\n"
+    )
+    sub_result.stdout = f"{js_string}{base_output}"
+    mocker.patch("reportseff.db_inquirer.subprocess.run", return_value=sub_result)
+    result = runner.invoke(
+        console.main,
+        [
+            "--no-color",
+            "4336165",
+        ],
+    )
+
+    assert result.exit_code == 0
+    # remove header
+    output = result.output.split("\n")[1:-1]
+    assert output[0].split() == [
+        "4336165",
+        "COMPLETED",
+        "00:18:59",
+        "21.1%",
+        "95.6%",
+        "46.1%",
+    ]
+    assert len(output) == 1
+
+    # this will return a different value for only the memory eff
+    no_memory = strip_js(js_string, ["used_memory", "total_memory"])
+    sub_result.stdout = f"{no_memory}{base_output}"
+    mocker.patch("reportseff.db_inquirer.subprocess.run", return_value=sub_result)
+    result = runner.invoke(
+        console.main,
+        [
+            "--no-color",
+            "4336165",
+        ],
+    )
+
+    assert result.exit_code == 0
+    # remove header
+    output = result.output.split("\n")[1:-1]
+    assert output[0].split() == [
+        "4336165",
+        "COMPLETED",
+        "00:18:59",
+        "21.1%",
+        "95.6%",
+        "100.0%",
+    ]
+    assert len(output) == 1
+
+    # this will return a different value for only the time eff
+    no_time = strip_js(js_string, ["total_time"])
+    sub_result.stdout = f"{no_time}{base_output}"
+    mocker.patch("reportseff.db_inquirer.subprocess.run", return_value=sub_result)
+    result = runner.invoke(
+        console.main,
+        [
+            "--no-color",
+            "4336165",
+        ],
+    )
+
+    assert result.exit_code == 0
+    # remove header
+    output = result.output.split("\n")[1:-1]
+    assert output[0].split() == [
+        "4336165",
+        "COMPLETED",
+        "00:18:59",
+        "21.1%",
+        "98.4%",
+        "46.1%",
+    ]
+    assert len(output) == 1
+
+    # this will return a different value for both the time and mem eff
+    no_time_mem = strip_js(js_string, ["used_memory", "total_memory", "total_time"])
+    sub_result.stdout = f"{no_time_mem}{base_output}"
+    mocker.patch("reportseff.db_inquirer.subprocess.run", return_value=sub_result)
+    result = runner.invoke(
+        console.main,
+        [
+            "--no-color",
+            "4336165",
+        ],
+    )
+
+    assert result.exit_code == 0
+    # remove header
+    output = result.output.split("\n")[1:-1]
+    assert output[0].split() == [
+        "4336165",
+        "COMPLETED",
+        "00:18:59",
+        "21.1%",
+        "98.4%",
+        "100.0%",
+    ]
+    assert len(output) == 1