fix(followups): F2 - Python entry points report an unreadable env file in one line
A .mam.env that exists but cannot be read or decoded stopped the Python
entry points with an EnvFileError traceback, while the wrapper printed one
`ERROR:` line and exited 1 (PR #43 review claude S1). The mains that load the
env file - publish_event, registry, job_subscriber, cli, bench, heartbeat and
remote - now call mqtt_common.load_env_or_report(), which prints
`ERROR: <path>: cannot read (<exception type>)` and lets the main return 1.
No value or file content is printed. ensure_env_loaded() still raises: no
library function exits the process.
Test: every entry point, for a 000 file and an undecodable one, ends with
exactly that line and rc 1 and creates nothing (14 cases; all 14 fail at
0051fd6 with a traceback).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
This commit is contained in:
1 parent
75936f11c6
commit
5edf913d22
9 files changed
+73
-9
No files matched your search
@@ -163,7 +163,8 @@ def run_benchmark(n=50, transport="fake", connect_latency_ms=50.0,
|
|||||||
|
|
||||||
|
|
||||||
def main(argv: Optional[List[str]] = None) -> int:
|
def main(argv: Optional[List[str]] = None) -> int:
|
||||||
mqtt_common.ensure_env_loaded()
|
if not mqtt_common.load_env_or_report():
|
||||||
|
return 1
|
||||||
parser = argparse.ArgumentParser(prog="lib_py.delegation.bench")
|
parser = argparse.ArgumentParser(prog="lib_py.delegation.bench")
|
||||||
parser.add_argument("--n", type=int, default=50)
|
parser.add_argument("--n", type=int, default=50)
|
||||||
parser.add_argument("--transport", default="fake", choices=["fake", "broker"])
|
parser.add_argument("--transport", default="fake", choices=["fake", "broker"])
|
||||||
|
|||||||
@@ -57,7 +57,8 @@ def _build_parser() -> argparse.ArgumentParser:
|
|||||||
|
|
||||||
|
|
||||||
def main(argv: Optional[List[str]] = None) -> int:
|
def main(argv: Optional[List[str]] = None) -> int:
|
||||||
mqtt_common.ensure_env_loaded()
|
if not mqtt_common.load_env_or_report():
|
||||||
|
return 1
|
||||||
args = _build_parser().parse_args(argv)
|
args = _build_parser().parse_args(argv)
|
||||||
|
|
||||||
if args.command == "submit":
|
if args.command == "submit":
|
||||||
|
|||||||
@@ -493,7 +493,8 @@ def main(argv=None) -> int:
|
|||||||
help="stop once this process (the agent) is gone")
|
help="stop once this process (the agent) is gone")
|
||||||
parser.add_argument("-v", "--verbose", action="store_true")
|
parser.add_argument("-v", "--verbose", action="store_true")
|
||||||
args = parser.parse_args(argv)
|
args = parser.parse_args(argv)
|
||||||
mqtt_common.ensure_env_loaded()
|
if not mqtt_common.load_env_or_report():
|
||||||
|
return 1
|
||||||
mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.INFO)
|
mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.INFO)
|
||||||
return run(args.job, args.registry_dir, args.interval, args.expect_dispatch_token,
|
return run(args.job, args.registry_dir, args.interval, args.expect_dispatch_token,
|
||||||
args.attempt_id, args.max_lifetime, args.watch_pid)
|
args.attempt_id, args.max_lifetime, args.watch_pid)
|
||||||
|
|||||||
@@ -587,7 +587,8 @@ def main(argv=None) -> int:
|
|||||||
parser.add_argument("-v", "--verbose", action="store_true")
|
parser.add_argument("-v", "--verbose", action="store_true")
|
||||||
args = parser.parse_args(argv)
|
args = parser.parse_args(argv)
|
||||||
|
|
||||||
mqtt_common.ensure_env_loaded()
|
if not mqtt_common.load_env_or_report():
|
||||||
|
return 1
|
||||||
mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.WARNING)
|
mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.WARNING)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ import hmac
|
|||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
import time
|
import time
|
||||||
import uuid
|
import uuid
|
||||||
@@ -110,9 +111,10 @@ def ensure_env_loaded(workspace_dir: Optional[str] = None) -> None:
|
|||||||
"""Load the workspace env file exactly once per process (D6, issue #5 phase 1).
|
"""Load the workspace env file exactly once per process (D6, issue #5 phase 1).
|
||||||
|
|
||||||
This used to run at import time. Importing a library must not mutate
|
This used to run at import time. Importing a library must not mutate
|
||||||
``os.environ``, so every *process entry point* calls this instead:
|
``os.environ``, so every *process entry point* calls this instead -
|
||||||
``publish_event.main``, ``registry.main``, ``job_subscriber.main``,
|
``publish_event.main``, ``registry.main``, ``job_subscriber.main``,
|
||||||
``cli.main``, ``bench.main``, and reconcile.sh's embedded subscriber.
|
``cli.main``, ``bench.main``, ``heartbeat.main`` and ``remote.main``
|
||||||
|
through :func:`load_env_or_report`, and reconcile.sh's embedded subscriber.
|
||||||
Production semantics are unchanged because every production path passes
|
Production semantics are unchanged because every production path passes
|
||||||
through one of those, and OS env still wins over the file
|
through one of those, and OS env still wins over the file
|
||||||
(see :func:`_parse_env_file`).
|
(see :func:`_parse_env_file`).
|
||||||
@@ -123,6 +125,20 @@ def ensure_env_loaded(workspace_dir: Optional[str] = None) -> None:
|
|||||||
_env_loaded = True
|
_env_loaded = True
|
||||||
_load_dotenv(workspace_dir)
|
_load_dotenv(workspace_dir)
|
||||||
|
|
||||||
|
|
||||||
|
def load_env_or_report() -> bool:
|
||||||
|
"""ensure_env_loaded() for a process entry point: an env file that exists
|
||||||
|
but cannot be read ends the process the way the wrapper does - one
|
||||||
|
`ERROR: <path>: cannot read (<exception type>)` line, no traceback, no
|
||||||
|
value - and the caller returns 1. False after printing; library code keeps
|
||||||
|
calling ensure_env_loaded() and never exits."""
|
||||||
|
try:
|
||||||
|
ensure_env_loaded()
|
||||||
|
except envfile.EnvFileError as e:
|
||||||
|
print(f"ERROR: {e}", file=sys.stderr)
|
||||||
|
return False
|
||||||
|
return True
|
||||||
|
|
||||||
# --------------------------------------------------------------------------
|
# --------------------------------------------------------------------------
|
||||||
# Constants
|
# Constants
|
||||||
# --------------------------------------------------------------------------
|
# --------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -334,7 +334,8 @@ def main(argv=None, _beat: bool = False) -> int:
|
|||||||
logger.error("--heartbeat needs a positive period and a non-terminal event")
|
logger.error("--heartbeat needs a positive period and a non-terminal event")
|
||||||
return 1
|
return 1
|
||||||
|
|
||||||
mqtt_common.ensure_env_loaded()
|
if not mqtt_common.load_env_or_report():
|
||||||
|
return 1
|
||||||
mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.WARNING)
|
mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.WARNING)
|
||||||
|
|
||||||
# --- parse optional data JSON (parameter error -> exit 1) ---
|
# --- parse optional data JSON (parameter error -> exit 1) ---
|
||||||
|
|||||||
@@ -533,7 +533,8 @@ def _prompt_from_args(parser: argparse.ArgumentParser, args: argparse.Namespace)
|
|||||||
|
|
||||||
|
|
||||||
def main(argv: Optional[List[str]] = None) -> int:
|
def main(argv: Optional[List[str]] = None) -> int:
|
||||||
mqtt_common.ensure_env_loaded()
|
if not mqtt_common.load_env_or_report():
|
||||||
|
return 1
|
||||||
mqtt_common.setup_logging(logging.INFO)
|
mqtt_common.setup_logging(logging.INFO)
|
||||||
parser = _build_parser()
|
parser = _build_parser()
|
||||||
args = parser.parse_args(argv)
|
args = parser.parse_args(argv)
|
||||||
|
|||||||
@@ -460,7 +460,8 @@ def main(argv=None) -> int:
|
|||||||
p_r.add_argument("--python", default=None)
|
p_r.add_argument("--python", default=None)
|
||||||
parser.add_argument("-v", "--verbose", action="store_true")
|
parser.add_argument("-v", "--verbose", action="store_true")
|
||||||
args = parser.parse_args(argv)
|
args = parser.parse_args(argv)
|
||||||
mqtt_common.ensure_env_loaded()
|
if not mqtt_common.load_env_or_report():
|
||||||
|
return 1
|
||||||
mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.INFO)
|
mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.INFO)
|
||||||
if args.command == "dispatch":
|
if args.command == "dispatch":
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -180,6 +180,47 @@ def test_a1_a_file_that_cannot_be_read_stops_both_loaders(tmp_path, kind):
|
|||||||
f.chmod(0o600)
|
f.chmod(0o600)
|
||||||
|
|
||||||
|
|
||||||
|
# Follow-up F2 (PR #43 claude S1): every main that loads the env file, with
|
||||||
|
# arguments its parser accepts when it parses before loading.
|
||||||
|
ENTRY_POINTS = {
|
||||||
|
"publish_event": ["--job", "abcd1234", "--event", "progress", "--registry-dir", "reg"],
|
||||||
|
"registry": [],
|
||||||
|
"job_subscriber": ["--job", "abcd1234", "--registry-dir", "reg"],
|
||||||
|
"cli": [],
|
||||||
|
"bench": [],
|
||||||
|
"heartbeat": ["--job", "abcd1234", "--interval", "30", "--registry-dir", "reg"],
|
||||||
|
"remote": ["run", "--session", "s", "--registry-dir", "reg"],
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("kind", ["permission", "utf8"])
|
||||||
|
@pytest.mark.parametrize("module", sorted(ENTRY_POINTS))
|
||||||
|
def test_a1_python_entry_points_report_an_unreadable_file_in_one_line(tmp_path, module, kind):
|
||||||
|
"""The wrapper's shape at every Python entry point: one `ERROR:` line
|
||||||
|
naming the path and the exception type, rc 1, no traceback, no value."""
|
||||||
|
f = tmp_path / "broken.env"
|
||||||
|
if kind == "permission":
|
||||||
|
if os.geteuid() == 0:
|
||||||
|
pytest.skip("root reads a 000 file")
|
||||||
|
f.write_text("PROBE_A=one\n")
|
||||||
|
f.chmod(0)
|
||||||
|
cause = "PermissionError"
|
||||||
|
else:
|
||||||
|
f.write_bytes(b"PROBE_A=one\nPROBE_B=\xff\xfe\n")
|
||||||
|
cause = "UnicodeDecodeError"
|
||||||
|
try:
|
||||||
|
# main() itself: `-m` adds runpy's warning (the package __init__ imports these modules)
|
||||||
|
code = f"import sys; from lib_py.delegation import {module} as m; sys.exit(m.main(sys.argv[1:]))"
|
||||||
|
res = subprocess.run([sys.executable, "-c", code, *ENTRY_POINTS[module]],
|
||||||
|
capture_output=True, text=True, env=_env(tmp_path, f), cwd=str(tmp_path), timeout=60)
|
||||||
|
finally:
|
||||||
|
f.chmod(0o600)
|
||||||
|
assert res.returncode == 1, (res.returncode, res.stderr[-500:])
|
||||||
|
assert res.stderr.strip().splitlines() == [f"ERROR: {f}: cannot read ({cause})"], res.stderr[-500:]
|
||||||
|
assert "one" not in res.stdout + res.stderr
|
||||||
|
assert not (tmp_path / "reg").exists(), "nothing ran past the env file"
|
||||||
|
|
||||||
|
|
||||||
def test_a1_a_nul_line_is_skipped_by_both_loaders(tmp_path):
|
def test_a1_a_nul_line_is_skipped_by_both_loaders(tmp_path):
|
||||||
"""Review R4: no environment can carry a NUL byte. Both loaders skip that
|
"""Review R4: no environment can carry a NUL byte. Both loaders skip that
|
||||||
line with a warning naming it (not its value) and keep the lines around it."""
|
line with a warning naming it (not its value) and keep the lines around it."""
|
||||||
|
|||||||
Reference in new issue
Block a user