Collect more system info#425
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #425 +/- ##
==========================================
+ Coverage 91.08% 91.24% +0.16%
==========================================
Files 15 15
Lines 1245 1268 +23
Branches 170 172 +2
==========================================
+ Hits 1134 1157 +23
Misses 77 77
Partials 34 34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
failing test on intel osx______________________ test_spawn_children[duct-1-plain] _______________________
temp_output_dir = '/private/var/folders/x7/_nsk_h4s1kng41z5pgfbx7sh0000gn/T/pytest-of-runner/pytest-0/test_spawn_children_duct_1_pla3/'
duct_cmd = 'duct', mode = 'plain', num_children = 1
@pytest.mark.flaky(reruns=3)
@pytest.mark.parametrize("mode", ["plain", "subshell", "nohup", "setsid"])
@pytest.mark.parametrize("num_children", [1, 2, 10])
def test_spawn_children(
temp_output_dir: str, duct_cmd: str, mode: str, num_children: int
) -> None:
duct_prefix = f"{temp_output_dir}log_"
script_path = TEST_SCRIPT_DIR / "spawn_children.sh"
dur = "0.3"
command = (
f"{duct_cmd} -q --s-i 0.001 --r-i 0.01 "
f"-p {duct_prefix} {script_path} {mode} {num_children} {dur}"
)
subprocess.check_output(command, shell=True)
with open(f"{duct_prefix}{SUFFIXES['usage']}") as usage_file:
all_samples = [json.loads(line) for line in usage_file]
# Only count the child sleep processes
all_child_pids = set(
pid
for sample in all_samples
for pid, proc in sample["processes"].items()
if "sleep" in proc["cmd"]
)
# Add one pid for the hold-the-door process, see spawn_children.sh line 7
if mode == "setsid":
assert len(all_child_pids) == 1
else:
> assert len(all_child_pids) == num_children + 1
E assert 0 == (1 + 1)
E + where 0 = len(set())
test/duct_main/test_e2e.py:60: AssertionError
=========================== short test summary info ============================
FAILED test/duct_main/test_e2e.py::test_spawn_children[duct-1-plain] - assert 0 == (1 + 1)
+ where 0 = len(set()) |
Adds 14 fields to the system block of info.json: 5 from platform.uname() (os_name, os_release, os_version, arch, processor) and 9 from platform.freedesktop_os_release() (distro_id, distro_id_like, distro_name, distro_version, distro_version_id, distro_codename, distro_variant_id, distro_pretty_name, distro_build_id). Bumps schema_version to 0.2.3. Distro fields fall back to empty strings on systems without /etc/os-release (e.g. macOS). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a 0.2.3 migration step to ensure_compliant_schema that fills the 14 new system.* provenance fields with empty strings when reading info.json files written by older duct versions, so ls and related consumers can rely on the fields being present. Updates test_ls fixtures to include a "system" block (which has existed in info.json since before MINIMUM_SCHEMA_VERSION) so the new migration can run against them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses review feedback: the bare `except OSError` was unexplained. The exception fires on systems without an os-release file (e.g. macOS), where the distro_* fields fall back to "". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3b05d93 to
bb4896c
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the info.json system block with OS + Linux distribution provenance (from platform.uname() and /etc/os-release) so downstream consumers can reliably branch on host metadata, and bumps the schema version from 0.2.2 to 0.2.3.
Changes:
- Add
os_*,arch,processor, anddistro_*fields toSystemInfoand collect them in the tracker. - Backfill new
systemfields when reading older logs vials.ensure_compliant_schema, and expose them as selectablecon-duct lsfields. - Update tests to include
systemand validate the new fields’ presence/types.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/test_ls.py | Updates fixtures to include system and asserts migration backfills new fields. |
| test/duct_main/test_report.py | Extends system-info sanity test to validate new OS/distro provenance fields. |
| src/con_duct/ls.py | Adds new fields to ls field choices and migrates pre-0.2.3 logs by backfilling system fields. |
| src/con_duct/_tracker.py | Collects uname + freedesktop OS-release metadata into SystemInfo. |
| src/con_duct/_models.py | Extends SystemInfo dataclass with OS/distro provenance fields. |
| src/con_duct/_constants.py | Bumps __schema_version__ to 0.2.3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # OS and distro provenance fields added to system block in 0.2.3 | ||
| if parse_version(info_dict["schema_version"]) < parse_version("0.2.3"): | ||
| for field in ( | ||
| "os_name", | ||
| "os_release", | ||
| "os_version", | ||
| "arch", | ||
| "processor", | ||
| "distro_id", | ||
| "distro_id_like", | ||
| "distro_name", | ||
| "distro_version", | ||
| "distro_version_id", | ||
| "distro_codename", | ||
| "distro_variant_id", | ||
| "distro_pretty_name", | ||
| "distro_build_id", | ||
| ): | ||
| info_dict["system"][field] = "" |
There was a problem hiding this comment.
system always exists on the oldest supported schema, and I dont think we should handle malformed logs at schema migration time. I think we should leave this as-is. If theres a missing field thats needed:
2026-06-01T11:06:26-0500 [WARNING ] con_duct.ls: Failed to load file <_io.TextIOWrapper name='info.json' mode='r' encoding='UTF-8'>: 'system'
Message could be improved, and ensure_compliant_schema could validate the required fields rather than just checking the version number, but thats a separate issue. Filed: #437
Summary
Adds OS and Linux-distribution provenance to the
systemblock ofinfo.json, so downstream consumers (plot, gallery, future tooling) can tell what host a run came from without parsing free-text strings.Schema bumped 0.2.2 → 0.2.3. Old logs read by
con-duct lsare backfilled with empty strings for the new fields, so consumers can rely on the fields being present.What's added (14 fields, all under
system)From
platform.uname():os_name,os_release,os_version,arch,processorFrom
platform.freedesktop_os_release()(Python 3.10+; reads/etc/os-release, falls back to""on systems without the file):distro_id,distro_id_like,distro_name,distro_version,distro_version_id,distro_codename,distro_variant_id,distro_pretty_name,distro_build_idSample
systemblock:Why so many fields?
The motivation here is provenance (because everything matters), not any single consumer's branching needs. Plot logic flagged the gap (it needs to branch between Linux and macOS for
pcpu), but capturing the standardos-releasefields now means future readers — gallery, repro tooling, archeology — can answer questions we haven't anticipated (variant, codename, build id) without us having to re-collect.Compatibility
0.2.3.ls.ensure_compliant_schemamigrates old logs by adding empty strings for all 14 fields, matching the convention set by the 0.2.1 (working_directory) and 0.2.2 (message) migrations.LS_FIELD_CHOICESextended so the new fields are queryable viacon-duct ls -Fand--eval-filter.os.sysconfcalls inget_system_info(). On systems without/etc/os-release(macOS), the ninedistro_*fields fall back to"".TODOs