Skip to content

Collect more system info#425

Open
asmacdo wants to merge 3 commits into
con:mainfrom
asmacdo:add-uname-to-info-json
Open

Collect more system info#425
asmacdo wants to merge 3 commits into
con:mainfrom
asmacdo:add-uname-to-info-json

Conversation

@asmacdo

@asmacdo asmacdo commented May 4, 2026

Copy link
Copy Markdown
Member

Summary

Adds OS and Linux-distribution provenance to the system block of info.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 ls are 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, processor

From 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_id

Sample system block:

 'system': {'arch': 'x86_64',
            'cpu_total': 20,
            'distro_build_id': '',
            'distro_codename': '',
            'distro_id': 'fedora',
            'distro_id_like': '',
            'distro_name': 'Fedora Linux',
            'distro_pretty_name': 'Fedora Linux 42 (Workstation Edition)',
            'distro_variant_id': 'workstation',
            'distro_version': '42 (Workstation Edition)',
            'distro_version_id': '42',
            'hostname': 'fancy',
            'memory_total': 33330028544,
            'os_name': 'Linux',
            'os_release': '6.19.12-100.fc42.x86_64',
            'os_version': '#1 SMP PREEMPT_DYNAMIC Sun Apr 12 15:27:03 UTC 2026',
            'processor': '',
            'uid': 1000,
            'user': 'austin'},

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 standard os-release fields 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

  • Schema version bumped to 0.2.3.
  • ls.ensure_compliant_schema migrates 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_CHOICES extended so the new fields are queryable via con-duct ls -F and --eval-filter.
  • POSIX-only, matching the existing constraint imposed by os.sysconf calls in get_system_info(). On systems without /etc/os-release (macOS), the nine distro_* fields fall back to "".

TODOs

  • fixup for the failing osx run one way or another

@codecov

codecov Bot commented May 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.24%. Comparing base (cbc6013) to head (bb4896c).
⚠️ Report is 5 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@asmacdo asmacdo added the semver-minor Increment the minor version when merged label May 5, 2026
Comment thread src/con_duct/_tracker.py
@yarikoptic

Copy link
Copy Markdown
Member
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())

asmacdo and others added 3 commits June 1, 2026 09:13
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>
@asmacdo asmacdo force-pushed the add-uname-to-info-json branch from 3b05d93 to bb4896c Compare June 1, 2026 14:39
@asmacdo asmacdo marked this pull request as ready for review June 1, 2026 15:01
Copilot AI review requested due to automatic review settings June 1, 2026 15:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, and distro_* fields to SystemInfo and collect them in the tracker.
  • Backfill new system fields when reading older logs via ls.ensure_compliant_schema, and expose them as selectable con-duct ls fields.
  • Update tests to include system and 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.

Comment thread src/con_duct/ls.py
Comment on lines +135 to +153
# 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] = ""

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-minor Increment the minor version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants