Fix plugin quality: schema-driven parsing, SIGTERM handling, container base alignment#4
Open
Fix plugin quality: schema-driven parsing, SIGTERM handling, container base alignment#4
Conversation
… bases
- Replace ~220 lines of manual _parse_* JSON-to-dataclass functions with
schema-driven deserialization using SDK's build_object_schema/unserialize.
Add _normalize_for_schema() recursive preprocessor that strips unknown
dict keys and coerces float→int for IntType fields before unserialize.
- Fix blocking default logic: change `if params.blocking is not False` to
`if params.blocking` and set schema default to True explicitly, making
intent clear and matching documented behavior.
- Add SIGTERM handler (_sigterm_handler) that forwards SIGTERM to the
active child process group, preventing orphaned ipc-benchmark processes
when the Arcaflow engine initiates graceful shutdown.
- Make benchmark timeout configurable via TestRunConfig.timeout (per-test),
replacing the hardcoded 3600s value. Error messages now report the
actual configured timeout.
- Fix _merge_outputs to aggregate duplicate mechanism summaries instead
of silently overwriting: total_messages are summed, best throughput
and lowest latencies are kept. Fastest/lowest-latency mechanism winners
are derived in a second pass over fully merged data.
- Improve type safety of BenchmarkResult.status: change from typing.Any
to str, add optional failure_reason field. Preprocessing in
_parse_json_output transforms {"Failure": "reason"} dicts into
status="Failure" + failure_reason="reason".
- Update arcalot base images from 0.4.0 to 0.5.0 for both buildbase
and osbase stages.
- Switch Rust build stage from Debian bookworm (glibc 2.36) to CentOS
Stream 9 (glibc 2.34) with rustup, matching the runtime base image
distro family and eliminating glibc forward-compatibility risk.
- Update tests: expect ConstraintException for missing keys, add tests
for float-to-int coercion, unknown key stripping, and extra top-level
keys.
AI-assisted-by: claude-4.6-opus (Anthropic)
Made-with: Cursor
…mpat - Remove explicit curl from dnf install in Rust build stage; the CentOS Stream 9 base already ships curl-minimal which conflicts with the full curl package but is sufficient for rustup download. - Bump arcaflow-plugin-sdk from ^0.14.0 to ^0.14.4 (resolves to 0.14.4) to fix ForwardRef._evaluate() TypeError on Python 3.12.4+. SDK 0.14.0 called ForwardRef._evaluate() without the recursive_guard keyword argument added in CPython 3.12.4. - Update Python version requirement from ^3.9 to ^3.12 to match SDK 0.14.4's minimum and the buildbase image's Python 3.12.11. - Regenerate poetry.lock with updated dependencies. AI-assisted-by: claude-4.6-opus (Anthropic) Made-with: Cursor
- Add ARG CACHE_BUST before the git clone step so the layer cache can be invalidated on demand (e.g. podman build --build-arg CACHE_BUST=$(date +%s)) without rebuilding the Rust toolchain. AI-assisted-by: claude-4.6-opus (Anthropic) Made-with: Cursor
- Change timeout field default from None to 3600 in TestRunConfig schema, making the default explicit (per review feedback). - Remove the None-check fallback in _run_single_test; since the schema always provides a value, test_config.timeout is used directly. AI-assisted-by: claude-4.6-opus (Anthropic) Made-with: Cursor
- Replace manual ARG CACHE_BUST with ADD of the GitHub API refs/heads/main endpoint. ADD always fetches the URL at build time; when the commit SHA changes the layer cache is invalidated automatically, forcing a fresh git clone and cargo build. - Works on both local and CI builds without requiring --build-arg. AI-assisted-by: claude-4.6-opus (Anthropic) Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses 9 quality and correctness issues in the rusty-comms Arcaflow plugin:
_parse_*functions with the SDK'sbuild_object_schema/unserialize, adding a recursive_normalize_for_schema()preprocessor that strips unknown keys and coerces float-to-int forIntTypefields.blockingschema default fromNonetoTrueand simplified the CLI condition fromis not Falseto a straightforward truthy check, making intent explicit._sigterm_handler()that forwards SIGTERM to the active child process group, preventing orphanedipc-benchmarkprocesses during Arcaflow engine shutdown.TestRunConfig.timeoutwith an explicit schema default of 3600s, replacing the hardcoded value. Error messages now report the actual configured timeout._merge_outputsnow aggregates duplicate mechanism summaries (sum messages, max throughput, min latencies) instead of silently overwriting. Winners are derived in a second pass over fully merged data.BenchmarkResult.statusfromtyping.Anytostr, addedfailure_reason: Optional[str]field. Preprocessing transforms{"Failure": "reason"}dicts from the Rust binary into the split typed fields.arcaflow-plugin-sdkfrom 0.14.0 to 0.14.4 and Python requirement from 3.9+ to 3.12+ to fixForwardRef._evaluate()TypeError on Python 3.12.4+.ARG CACHE_BUSTbeforegit cloneso the layer cache can be invalidated on demand without rebuilding the Rust toolchain.Test plan
podman buildsucceeds end-to-end (Rust build on CentOS Stream 9 + Python tests in buildbase)ipc-benchmarkbinary to verify JSON parsing with real outputMade with Cursor