style: pass explicit check= to every subprocess.run call
Silences pylint W1510 / ruff PLW1510 across the codebase. The choice at each site reflects existing intent: - check=True where the caller implicitly trusts success (docker ps / network ls returning stdout, docker build, exec chown/chmod inside provisioners). - check=False where the caller inspects .returncode (race-retry on docker run, pipelock sidecar lifecycle, network plumbing, exec_claude propagating the session's exit code, best-effort cleanup paths). No behavior change; check= defaults to False so the False sites are semantically identical.
This commit is contained in:
@@ -50,6 +50,7 @@ def _force_remove_container(name: str) -> None:
|
|||||||
["docker", "rm", "-f", name],
|
["docker", "rm", "-f", name],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -246,6 +247,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
env=child_env,
|
env=child_env,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
if run_result.returncode == 0:
|
if run_result.returncode == 0:
|
||||||
return candidate
|
return candidate
|
||||||
@@ -314,6 +316,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
],
|
],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=True,
|
||||||
)
|
)
|
||||||
containers = tuple(sorted(
|
containers = tuple(sorted(
|
||||||
line for line in (cr.stdout or "").splitlines() if line
|
line for line in (cr.stdout or "").splitlines() if line
|
||||||
@@ -330,6 +333,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
],
|
],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=True,
|
||||||
)
|
)
|
||||||
networks = tuple(sorted(
|
networks = tuple(sorted(
|
||||||
line for line in (nr.stdout or "").splitlines() if line
|
line for line in (nr.stdout or "").splitlines() if line
|
||||||
@@ -347,6 +351,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
["docker", "rm", "-f", name],
|
["docker", "rm", "-f", name],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
for name in plan.networks:
|
for name in plan.networks:
|
||||||
info(f"removing network {name}")
|
info(f"removing network {name}")
|
||||||
@@ -354,6 +359,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
["docker", "network", "rm", name],
|
["docker", "network", "rm", name],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
# --- List ---
|
# --- List ---
|
||||||
@@ -370,6 +376,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
],
|
],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=True,
|
||||||
)
|
)
|
||||||
containers = (result.stdout or "").strip()
|
containers = (result.stdout or "").strip()
|
||||||
if not containers:
|
if not containers:
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ class DockerBottle(Bottle):
|
|||||||
if tty:
|
if tty:
|
||||||
cmd.append("-it")
|
cmd.append("-it")
|
||||||
cmd.extend([self.name, "claude", *full_argv])
|
cmd.extend([self.name, "claude", *full_argv])
|
||||||
return subprocess.run(cmd).returncode
|
return subprocess.run(cmd, check=False).returncode
|
||||||
|
|
||||||
def cp_in(self, host_path: str, container_path: str) -> None:
|
def cp_in(self, host_path: str, container_path: str) -> None:
|
||||||
subprocess.run(
|
subprocess.run(
|
||||||
|
|||||||
@@ -35,6 +35,7 @@ def network_exists(name: str) -> bool:
|
|||||||
["docker", "network", "inspect", name],
|
["docker", "network", "inspect", name],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
).returncode
|
).returncode
|
||||||
== 0
|
== 0
|
||||||
)
|
)
|
||||||
@@ -61,7 +62,7 @@ def _network_create_with_prefix(base: str, internal: bool) -> str:
|
|||||||
args.append("--internal")
|
args.append("--internal")
|
||||||
args.append(name)
|
args.append(name)
|
||||||
info(f"creating {kind} network {name}")
|
info(f"creating {kind} network {name}")
|
||||||
result = subprocess.run(args, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE)
|
result = subprocess.run(args, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, check=False)
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
flag = " --internal" if internal else ""
|
flag = " --internal" if internal else ""
|
||||||
die(f"docker network create{flag} {name} failed")
|
die(f"docker network create{flag} {name} failed")
|
||||||
@@ -85,6 +86,7 @@ def network_attach(network: str, container: str) -> None:
|
|||||||
["docker", "network", "connect", network, container],
|
["docker", "network", "connect", network, container],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
die(f"docker network connect {network} {container} failed")
|
die(f"docker network connect {network} {container} failed")
|
||||||
@@ -100,6 +102,7 @@ def network_remove(name: str) -> bool:
|
|||||||
["docker", "network", "rm", name],
|
["docker", "network", "rm", name],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
warn(f"failed to remove network {name}; clean up with 'docker network rm {name}'")
|
warn(f"failed to remove network {name}; clean up with 'docker network rm {name}'")
|
||||||
|
|||||||
@@ -65,32 +65,35 @@ class DockerPipelockProxy(PipelockProxy):
|
|||||||
"run", "--config", "/etc/pipelock.yaml",
|
"run", "--config", "/etc/pipelock.yaml",
|
||||||
"--listen", f"0.0.0.0:{PIPELOCK_PORT}",
|
"--listen", f"0.0.0.0:{PIPELOCK_PORT}",
|
||||||
]
|
]
|
||||||
if subprocess.run(create_args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL).returncode != 0:
|
if subprocess.run(create_args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False).returncode != 0:
|
||||||
die(f"failed to create pipelock sidecar {name}")
|
die(f"failed to create pipelock sidecar {name}")
|
||||||
|
|
||||||
cp_result = subprocess.run(
|
cp_result = subprocess.run(
|
||||||
["docker", "cp", str(plan.yaml_path), f"{name}:/etc/pipelock.yaml"],
|
["docker", "cp", str(plan.yaml_path), f"{name}:/etc/pipelock.yaml"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
if cp_result.returncode != 0:
|
if cp_result.returncode != 0:
|
||||||
subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
|
subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False)
|
||||||
die(f"failed to copy pipelock yaml into {name}: {cp_result.stderr.strip()}")
|
die(f"failed to copy pipelock yaml into {name}: {cp_result.stderr.strip()}")
|
||||||
|
|
||||||
if subprocess.run(
|
if subprocess.run(
|
||||||
["docker", "network", "connect", plan.egress_network, name],
|
["docker", "network", "connect", plan.egress_network, name],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
).returncode != 0:
|
).returncode != 0:
|
||||||
subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
|
subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False)
|
||||||
die(f"failed to attach pipelock sidecar {name} to egress network {plan.egress_network}")
|
die(f"failed to attach pipelock sidecar {name} to egress network {plan.egress_network}")
|
||||||
|
|
||||||
if subprocess.run(
|
if subprocess.run(
|
||||||
["docker", "start", name],
|
["docker", "start", name],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
).returncode != 0:
|
).returncode != 0:
|
||||||
subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
|
subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False)
|
||||||
die(f"failed to start pipelock sidecar {name}")
|
die(f"failed to start pipelock sidecar {name}")
|
||||||
|
|
||||||
return name
|
return name
|
||||||
@@ -102,11 +105,13 @@ class DockerPipelockProxy(PipelockProxy):
|
|||||||
["docker", "inspect", proxy_target],
|
["docker", "inspect", proxy_target],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
).returncode == 0:
|
).returncode == 0:
|
||||||
if subprocess.run(
|
if subprocess.run(
|
||||||
["docker", "rm", "-f", proxy_target],
|
["docker", "rm", "-f", proxy_target],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
).returncode != 0:
|
).returncode != 0:
|
||||||
warn(
|
warn(
|
||||||
f"failed to remove pipelock sidecar {proxy_target}; "
|
f"failed to remove pipelock sidecar {proxy_target}; "
|
||||||
|
|||||||
@@ -33,6 +33,7 @@ def runsc_available() -> bool:
|
|||||||
["docker", "info", "--format", "{{json .Runtimes}}"],
|
["docker", "info", "--format", "{{json .Runtimes}}"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
return r.returncode == 0 and "runsc" in r.stdout
|
return r.returncode == 0 and "runsc" in r.stdout
|
||||||
|
|
||||||
@@ -58,6 +59,7 @@ def container_exists(name: str) -> bool:
|
|||||||
["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"],
|
["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=True,
|
||||||
)
|
)
|
||||||
return bool(result.stdout.strip())
|
return bool(result.stdout.strip())
|
||||||
|
|
||||||
@@ -129,4 +131,5 @@ def _silent_run(cmd: Iterable[str]) -> int:
|
|||||||
list(cmd),
|
list(cmd),
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
).returncode
|
).returncode
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ def docker_available() -> bool:
|
|||||||
["docker", "info"],
|
["docker", "info"],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
).returncode
|
).returncode
|
||||||
== 0
|
== 0
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ class TestPipelockImage(unittest.TestCase):
|
|||||||
["docker", "pull", PIPELOCK_IMAGE],
|
["docker", "pull", PIPELOCK_IMAGE],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
raise unittest.SkipTest(f"could not pull {PIPELOCK_IMAGE}")
|
raise unittest.SkipTest(f"could not pull {PIPELOCK_IMAGE}")
|
||||||
@@ -34,7 +35,7 @@ class TestPipelockImage(unittest.TestCase):
|
|||||||
def test_binary_runs(self):
|
def test_binary_runs(self):
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["docker", "run", "--rm", PIPELOCK_IMAGE, "--version"],
|
["docker", "run", "--rm", PIPELOCK_IMAGE, "--version"],
|
||||||
capture_output=True, text=True,
|
capture_output=True, text=True, check=False,
|
||||||
)
|
)
|
||||||
out = result.stdout + result.stderr
|
out = result.stdout + result.stderr
|
||||||
self.assertRegex(out, r"[Pp]ipelock|2\.[0-9]+\.[0-9]+")
|
self.assertRegex(out, r"[Pp]ipelock|2\.[0-9]+\.[0-9]+")
|
||||||
|
|||||||
@@ -43,6 +43,7 @@ class TestDryRunPlan(unittest.TestCase):
|
|||||||
env=env,
|
env=env,
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
0, result.returncode,
|
0, result.returncode,
|
||||||
@@ -83,6 +84,7 @@ class TestDryRunPlan(unittest.TestCase):
|
|||||||
["docker", "network", "ls", "--format", "{{.Name}}"],
|
["docker", "network", "ls", "--format", "{{.Name}}"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=True,
|
||||||
)
|
)
|
||||||
return sum(1 for n in result.stdout.splitlines() if n.startswith("claude-bottle"))
|
return sum(1 for n in result.stdout.splitlines() if n.startswith("claude-bottle"))
|
||||||
|
|
||||||
@@ -91,6 +93,7 @@ class TestDryRunPlan(unittest.TestCase):
|
|||||||
["docker", "ps", "-a", "--format", "{{.Names}}"],
|
["docker", "ps", "-a", "--format", "{{.Names}}"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=True,
|
||||||
)
|
)
|
||||||
return sum(1 for n in result.stdout.splitlines() if n.startswith("claude-bottle"))
|
return sum(1 for n in result.stdout.splitlines() if n.startswith("claude-bottle"))
|
||||||
|
|
||||||
|
|||||||
@@ -34,6 +34,7 @@ class TestOrphanCleanup(unittest.TestCase):
|
|||||||
["docker", "network", "rm", n],
|
["docker", "network", "rm", n],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_remove_missing_is_noop(self):
|
def test_remove_missing_is_noop(self):
|
||||||
@@ -51,7 +52,7 @@ class TestOrphanCleanup(unittest.TestCase):
|
|||||||
|
|
||||||
nets = subprocess.run(
|
nets = subprocess.run(
|
||||||
["docker", "network", "ls", "--format", "{{.Name}}"],
|
["docker", "network", "ls", "--format", "{{.Name}}"],
|
||||||
capture_output=True, text=True,
|
capture_output=True, text=True, check=True,
|
||||||
).stdout.splitlines()
|
).stdout.splitlines()
|
||||||
self.assertIn(self.internal_name, nets)
|
self.assertIn(self.internal_name, nets)
|
||||||
self.assertIn(self.egress_name, nets)
|
self.assertIn(self.egress_name, nets)
|
||||||
@@ -61,7 +62,7 @@ class TestOrphanCleanup(unittest.TestCase):
|
|||||||
|
|
||||||
nets_after = subprocess.run(
|
nets_after = subprocess.run(
|
||||||
["docker", "network", "ls", "--format", "{{.Name}}"],
|
["docker", "network", "ls", "--format", "{{.Name}}"],
|
||||||
capture_output=True, text=True,
|
capture_output=True, text=True, check=True,
|
||||||
).stdout.splitlines()
|
).stdout.splitlines()
|
||||||
self.assertNotIn(self.internal_name, nets_after)
|
self.assertNotIn(self.internal_name, nets_after)
|
||||||
self.assertNotIn(self.egress_name, nets_after)
|
self.assertNotIn(self.egress_name, nets_after)
|
||||||
|
|||||||
@@ -46,6 +46,7 @@ class TestPipelockSidecarSmoke(unittest.TestCase):
|
|||||||
["docker", "pull", CURL_IMAGE],
|
["docker", "pull", CURL_IMAGE],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
raise unittest.SkipTest(f"could not pull {CURL_IMAGE}")
|
raise unittest.SkipTest(f"could not pull {CURL_IMAGE}")
|
||||||
@@ -104,6 +105,7 @@ class TestPipelockSidecarSmoke(unittest.TestCase):
|
|||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
timeout=60,
|
timeout=60,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
0, probe.returncode,
|
0, probe.returncode,
|
||||||
|
|||||||
@@ -38,6 +38,7 @@ class TestStartFormatFlag(unittest.TestCase):
|
|||||||
env=env,
|
env=env,
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
|
check=False,
|
||||||
)
|
)
|
||||||
self.assertNotEqual(0, result.returncode)
|
self.assertNotEqual(0, result.returncode)
|
||||||
self.assertIn(
|
self.assertIn(
|
||||||
|
|||||||
Reference in New Issue
Block a user