Skip to content

various issues to address #2

@mcurrier2

Description

@mcurrier2

Manual JSON-to-dataclass parsing: The parse* functions (lines 148-370) manually map every JSON field to dataclass constructors. Other arcalot plugins that wrap tools producing JSON typically rely on simpler approaches or the SDK's deserialization. This is ~220 lines of boilerplate that could break if the rusty-comms JSON format changes. There's no schema validation layer — just KeyError if a field is missing.

blocking default logic is fragile (line 104):                                                                                                                            
if params.blocking is not False:
args.append("--blocking")This means blocking=None (unset) adds --blocking. The intent is documented, but it's an implicit default that diverges from what the schema says ("Defaults to true when not specified"). If the rusty-comms binary itself defaults to blocking, this double-applies it.

No SIGTERM handler on the plugin itself: The arcalot protocol gives plugins up to 30 seconds to shut down on SIGTERM. This plugin handles SIGTERM for child processes but doesn't register a signal handler for the plugin process itself to forward SIGTERM to running children. If the Arcaflow engine sends SIGTERM to the plugin, the child process group could be orphaned.

Hardcoded 3600s timeout (line 382): The timeout isn't configurable via InputParams. A user running a long benchmark matrix has no way to extend it, and the error message at line 529 hardcodes "3600 seconds" rather than reading the actual value.

_merge_outputs picks winners naively (lines 622-643): It iterates outputs sequentially and picks the fastest/lowest-latency mechanism by comparing across runs, but all_mechanisms.update() at line 620 silently overwrites duplicate mechanism keys if two test runs benchmark the same mechanism. The last run wins.

status field typed as typing.Any (schema line 572): This accepts anything — a string "Success" or a dict {"Failure": "reason"}. A discriminated union or enum would be more type-safe and would generate a better Arcaflow schema.

Stale base image versions: This plugin uses 0.4.0 for both container base images. These should be updated to the latest 0.5.0 version.                                                                                                                                          

Debian vs RHEL base: The Rust stage uses slim-bookworm (Debian). The arcalot base images are RHEL/CentOS-based. While the compiled binary is statically linked enough to work, mixing distro families in a build is unusual.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions