Skip to content

build(deps): bump typeguard from 2.13.3 to 4.6.0 #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
galacto-last9 merged 4 commits into from
Sep 18, 2026
Merged

build(deps): bump typeguard from 2.13.3 to 4.6.0 #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a195845
build(deps): bump typeguard from 2.13.3 to 4.6.0
dependabot[bot] Aug 2, 2026
49bf44e
test: adapt health check doubles to Typeguard 4 protocols
galacto Sep 18, 2026
d2b0364
fix: align shell command results with runtime protocol checks
galacto Sep 18, 2026
38ca2b7
test: isolate subprocess exit status from Slurm child reaper
galacto Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions dev-requirements.txt
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions l9gpu/health_checks/subprocess.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
# Copyright (c) Last9, Inc.
import subprocess
from dataclasses import dataclass
from typing import Dict, List, Optional, Protocol
from typing import Dict, List, Optional, Protocol, Union


class ShellCommandOut(Protocol):
args: List[str]
args: Union[str, List[str]]
returncode: int
stdout: str
stderr: str
stderr: Optional[str]

def check_returncode(self) -> None: ...

Expand All @@ -23,7 +23,7 @@ class PipedShellCommandOut:
def handle_subprocess_exception(exc: Exception) -> ShellCommandOut:
if isinstance(exc, subprocess.TimeoutExpired):
return subprocess.CompletedProcess(
args=[exc.cmd],
args=exc.cmd,
returncode=128,
stdout="Error command timeout because of timeout setting.\n",
)
Expand Down
4 changes: 2 additions & 2 deletions l9gpu/tests/health_checks_tests/test_check_sensors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ class FakeSensorsCheckImpl:

def get_sensors(
self,
_timeout_secs: int,
_logger: logging.Logger,
timeout_secs: int,
logger: logging.Logger,
) -> ShellCommandOut:
"""Return pregenerated output instead of calling ipmi-sensors."""
return self.sensors_out
Expand Down
4 changes: 2 additions & 2 deletions l9gpu/tests/health_checks_tests/test_check_ssh_certs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class FakeSshCertsCheckImpl:
log_level: str = "INFO"
log_folder: str = "/tmp"

def get_ipa_certs(self, _host: str, timeout_secs: int) -> ShellCommandOut:
def get_ipa_certs(self, host: str, timeout_secs: int) -> ShellCommandOut:
"""
Return first param instead of invoking ipa host-show.

Expand All @@ -62,7 +62,7 @@ def get_ipa_certs(self, _host: str, timeout_secs: int) -> ShellCommandOut:
)
return self.params[0]

def get_ssh_certs(self, _host: str, timeout_secs: int) -> ShellCommandOut:
def get_ssh_certs(self, host: str, timeout_secs: int) -> ShellCommandOut:
"""
Return second param instead of invoking ssh-keyscan and ssh-keygen.

Expand Down
112 changes: 26 additions & 86 deletions l9gpu/tests/health_checks_tests/test_check_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
from l9gpu.tests.fakes import FakeShellCommandOut


@dataclass
class FakeCheckDiskStorageImpl:
disk_usage: ShellCommandOut
class FakeStorageCheck:
"""Implement the full storage protocol without running host commands.

Each test overrides the operations it exercises; unexpected calls fail.
"""

cluster = "test cluster"
type = "prolog"
Expand All @@ -27,30 +29,38 @@ class FakeCheckDiskStorageImpl:
def get_disk_usage(
self, timeout_secs: int, volume: str, logger: logging.Logger
) -> ShellCommandOut:
return self.disk_usage
raise NotImplementedError

def get_mount_status(
self, timeout_secs: int, dir: str, logger: logging.Logger
) -> PipedShellCommandOut:
return PipedShellCommandOut([0, 0], "dummy output")
raise NotImplementedError

def check_file_exists(self, f: str, logger: logging.Logger) -> bool:
return True
raise NotImplementedError

def check_directory_exists(self, dir: str, logger: logging.Logger) -> bool:
return True
raise NotImplementedError

def get_fstab_mount_info(
self, timeout_secs: int, mountpoint: str, logger: logging.Logger
) -> Tuple[ShellCommandOut, ShellCommandOut]:
return FakeShellCommandOut([""], 0, "dummy output"), FakeShellCommandOut(
[""], 0, "dummy output"
)
raise NotImplementedError

def get_disk_size(
self, timeout_secs: int, volume: str, units: str, logger: logging.Logger
) -> PipedShellCommandOut:
return PipedShellCommandOut([0], "")
raise NotImplementedError


@dataclass
class FakeCheckDiskStorageImpl(FakeStorageCheck):
disk_usage: ShellCommandOut

def get_disk_usage(
self, timeout_secs: int, volume: str, logger: logging.Logger
) -> ShellCommandOut:
return self.disk_usage


@pytest.fixture
Expand Down Expand Up @@ -212,30 +222,14 @@ def test_inode_usage(


@dataclass
class FakeCheckMountImpl:
class FakeCheckMountImpl(FakeStorageCheck):
mount_status: PipedShellCommandOut

cluster = "test cluster"
type = "prolog"
log_level = "INFO"
log_folder = "/tmp"

def get_disk_usage(
self, timeout_secs: int, volume: str, logger: logging.Logger
) -> ShellCommandOut:
return FakeShellCommandOut([""], 0, "dummy output")

def get_mount_status(
self, timeout_secs: int, dir: str, logger: logging.Logger
) -> PipedShellCommandOut:
return self.mount_status

def check_file_exists(self, f: str, logger: logging.Logger) -> bool:
return True

def check_directory_exists(self, dir: str, logger: logging.Logger) -> bool:
return True


@pytest.fixture
def mount_tester(request: pytest.FixtureRequest) -> FakeCheckMountImpl:
Expand Down Expand Up @@ -297,37 +291,15 @@ def test_mounted_directory(


@dataclass
class FakeCheckExistanceImpl:
class FakeCheckExistanceImpl(FakeStorageCheck):
existance: bool

cluster = "test cluster"
type = "prolog"
log_level = "INFO"
log_folder = "/tmp"

def get_disk_usage(
self, timeout_secs: int, volume: str, logger: logging.Logger
) -> ShellCommandOut:
return FakeShellCommandOut([""], 0, "dummy output")

def get_mount_status(
self, timeout_secs: int, dir: str, logger: logging.Logger
) -> PipedShellCommandOut:
return PipedShellCommandOut([0, 0], "dummy output")

def check_file_exists(self, f: str, logger: logging.Logger) -> bool:
return self.existance

def check_directory_exists(self, dir: str, logger: logging.Logger) -> bool:
return self.existance

def get_fstab_mount_info(
self, timeout_secs: int, mountpoint: str, logger: logging.Logger
) -> Tuple[ShellCommandOut, ShellCommandOut]:
return FakeShellCommandOut([""], 0, "dummy output"), FakeShellCommandOut(
[""], 0, "dummy output"
)


@pytest.fixture
def existance_tester(request: pytest.FixtureRequest) -> FakeCheckExistanceImpl:
Expand Down Expand Up @@ -453,11 +425,7 @@ def test_dir_existance(


@dataclass
class FakeExceptionImpl:
cluster = "test cluster"
type = "prolog"
log_level = "INFO"
log_folder = "/tmp"
class FakeExceptionImpl(FakeStorageCheck):

def get_disk_usage(
self, timeout_secs: int, volume: str, logger: logging.Logger
Expand All @@ -475,13 +443,6 @@ def check_file_exists(self, f: str, logger: logging.Logger) -> bool:
def check_directory_exists(self, dir: str, logger: logging.Logger) -> bool:
raise ValueError("Some error")

def get_fstab_mount_info(
self, timeout_secs: int, mountpoint: str, logger: logging.Logger
) -> Tuple[ShellCommandOut, ShellCommandOut]:
return FakeShellCommandOut([""], 0, "dummy output"), FakeShellCommandOut(
[""], 0, "dummy output"
)


def test_file_dir_exception(
caplog: pytest.LogCaptureFixture,
Expand Down Expand Up @@ -517,30 +478,9 @@ def test_file_dir_exception(


@dataclass
class FakeCheckMountpointImpl:
class FakeCheckMountpointImpl(FakeStorageCheck):
mountpoint: Tuple[ShellCommandOut, ShellCommandOut]

cluster = "test cluster"
type = "prolog"
log_level = "INFO"
log_folder = "/tmp"

def get_disk_usage(
self, timeout_secs: int, volume: str, logger: logging.Logger
) -> ShellCommandOut:
return FakeShellCommandOut([""], 0, "dummy output")

def get_mount_status(
self, timeout_secs: int, dir: str, logger: logging.Logger
) -> PipedShellCommandOut:
return PipedShellCommandOut([0, 0], "dummy output")

def check_file_exists(self, f: str, logger: logging.Logger) -> bool:
return True

def check_directory_exists(self, dir: str, logger: logging.Logger) -> bool:
return True

def get_fstab_mount_info(
self, timeout_secs: int, mountpoint: str, logger: logging.Logger
) -> Tuple[ShellCommandOut, ShellCommandOut]:
Expand Down Expand Up @@ -630,7 +570,7 @@ def test_check_mountpoint(


@dataclass
class FakeDiskSizeImpl:
class FakeDiskSizeImpl(FakeStorageCheck):
def get_disk_size(
self, timeout_secs: int, volume: str, units: str, logger: logging.Logger
) -> PipedShellCommandOut:
Expand Down
61 changes: 61 additions & 0 deletions l9gpu/tests/health_checks_tests/test_subprocess.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Copyright (c) Last9, Inc.
import shlex
import signal
import subprocess
import sys

import pytest
from typeguard import check_type

from l9gpu.health_checks.subprocess import (
handle_subprocess_exception,
shell_command,
ShellCommandOut,
)


@pytest.mark.parametrize("returncode", [0, 2])
def test_shell_command_result_matches_protocol(returncode: int) -> None:
cmd = shlex.join(
[
sys.executable,
"-c",
"import sys; print('output'); print('error', file=sys.stderr); "
f"sys.exit({returncode})",
]
)

# The sacct_backfill CLI installs a child-reaping handler at import time.
# Let subprocess collect its own exit status, regardless of test order.
previous_handler = signal.signal(signal.SIGCHLD, signal.SIG_DFL)
try:
result = shell_command(cmd, timeout_secs=10)
finally:
signal.signal(signal.SIGCHLD, previous_handler)

check_type(result, ShellCommandOut)
assert result.args == cmd
assert result.returncode == returncode
assert set(result.stdout.splitlines()) == {"output", "error"}
assert result.stderr is None


@pytest.mark.parametrize("cmd", ["test-command --flag", ["test-command", "--flag"]])
def test_timeout_result_matches_protocol(cmd: str | list[str]) -> None:
result = handle_subprocess_exception(subprocess.TimeoutExpired(cmd, 10))

check_type(result, ShellCommandOut)
assert result.args == cmd
assert result.returncode == 128
assert "Error command timeout because of timeout setting." in result.stdout
with pytest.raises(subprocess.CalledProcessError):
result.check_returncode()


def test_unknown_exception_result_matches_protocol() -> None:
result = handle_subprocess_exception(ValueError("unexpected failure"))

check_type(result, ShellCommandOut)
assert result.args == []
assert result.returncode == 2
assert "Unknown subprocess exception" in result.stdout
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ dependencies = [
"pynvml==11.4.*",
"typing_extensions",
"click==8.0.4",
"typeguard==2.13.3",
"typeguard==4.6.0",

@nishantmodak nishantmodak Sep 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Migrate subprocess result contracts before enabling typeguard 4

This upgrade instruments annotated local assignments and validates protocol attributes. The real DCGMImpl returns shell_command() results whose CompletedProcess.args is a string and stderr is None (shell=True, stderr=STDOUT), but ShellCommandOut requires List[str] and str. At check_dcgmi.py:461, a normal command result now raises TypeCheckError; the exception recovery result then fails the same protocol's stderr check. The timeout path fails as well.

I reproduced this through the actual CLI, DCGMImpl, shell wrapper and output contexts with only the subprocess boundary replaced. A passing GPU diagnostic, a failing diagnostic, and a timeout all end with WARNING - check did not exit normally and TypeCheckError, instead of their expected OK/CRITICAL/WARN result and diagnostic message. This breaks the health-check path even for valid command output and can turn a critical result into an unexplained warning.

Update the subprocess protocol and recovery values to reflect the real producer contract, and audit the newly enforced annotated assignments and protocol consumers before taking the major upgrade. Keep this dependency at its compatible version until those migrations and the affected tests pass. Do not solve this only by changing test fakes: the production CompletedProcess shape triggers the failure.

Executed regression at a195845b8c708b44eb5876d1911ec06f074a9908: save the following as l9gpu/tests/test_typeguard_subprocess_review.py. With Python 3.12 and the PR's declared dependencies installed (python -m pip install -e '.[dev,k8s]'), run:

python -m pytest -q l9gpu/tests/test_typeguard_subprocess_review.py
import json
import subprocess
from unittest.mock import patch

import pytest
from click.testing import CliRunner
from l9gpu.health_checks.checks.check_dcgmi import DCGMImpl, check_dcgmi


@pytest.mark.parametrize("status,code,message", [
    ("Pass", 0, "All checks passed"),
    ("Fail", 2, "software failed"),
    ("timeout", 1, "Error command timeout because of timeout setting"),
])
def test_real_subprocess_contract(tmp_path, status, code, message):
    def fake_run(cmd, **kwargs):
        # Same CompletedProcess shape as the production shell=True /
        # stderr=STDOUT / text-mode subprocess.run invocation.
        assert kwargs["shell"] is True
        assert kwargs["stderr"] is subprocess.STDOUT
        if status == "timeout":
            raise subprocess.TimeoutExpired(cmd, kwargs["timeout"])
        body = {"DCGM Diagnostic": {"test_categories": [
            {"tests": [{"name": "software", "test_summary": {"status": status}}]}
        ]}}
        return subprocess.CompletedProcess(cmd, 0, json.dumps(body), None)

    obj = DCGMImpl("synthetic", "nagios", "INFO", str(tmp_path), "localhost")
    with patch("l9gpu.health_checks.subprocess.subprocess.run", side_effect=fake_run):
        result = CliRunner().invoke(check_dcgmi, [
            "diag", "synthetic", "nagios", "--log-folder", str(tmp_path),
            "--sink", "do_nothing", "--verbose-out",
        ], obj=obj)
    assert result.exit_code == code, (result.output, repr(result.exception))
    assert message in result.output, (result.output, repr(result.exception))

Executed on Python 3.12.9: all three cases fail on this head. The identical test passes all three at base 215314527174c7e59743f21fde067be26db5fff1 with typeguard 2.13.3 and otherwise identical resolved dependencies. The failure assertions show TypeCheckError on the ShellCommandOut attributes. No GPU, subprocess command, service, or external telemetry endpoint is used by this test.

"click-option-group",
"pydantic==2.13.4",
"omegaconf~=2.2",
Expand Down
Loading