Repository: conbench/conbench (language: python)

Issue:
benchadapt: Remove BenchmarkRun class
As we move towards removing the Run table (see #1227), the `/api/runs` POST endpoint will be removed. The [`BenchmarkRun` class from benchadapt](https://github.com/conbench/conbench/blob/2411b0efb70036dc3563c19b12f6bd3ffd482594/benchadapt/python/benchadapt/run.py) reflects this endpoint, and should be removed.

Part of this story is tracking any usages of this class and taking steps towards mitigating them. (Almost all versions are pinned, so changes here can be merged before fixes if necessary.) Already known: benchconnect (#1429)
benchconnect: Remove run posting and BenchmarkRun usage
Closes #1429 by removing the POST and PUT calls to the `api/runs` endpoints, plus removes all usages of benchadapt's `BenchmarkRun` object so that can be removed. Removes functionality around runs, but it shouldn't change any current user workflows.

I want to test the `start`/`submit`/`finish` workflow against a local Conbench instance before merging because we don't have integration tests for that yet, but I don't expect that to change what's here much if at all.


Discussion / hints:
Good thinking. I want to say that for achieving our high-prio goals and for having less constraints to manage, we can take the tension / stress out of this by making `/api/runs POST` be a noop on the API implementation side, at least for a transition time. It's a typical strategy buying peace of mind.

A clear migration path with no parts obviously breaking already requires managing a dependency tree by doing specific changes in a certain order. That can be tough to plan. The more simple this dependency tree can be made the less likely are fatal oversights.

think: for achieving our actual goals we do not need to break clients yet that _try to_ POST to /api/runs; this does not need to be part of the 'first transaction'.


Cool, that makes it easier to deal with this when we've got capacity. The exception is if the client does something with the response or hits the API for it later, but benchconnect doesn't:

https://github.com/conbench/conbench/blob/b03b1ffa0716098143ac0cbe354aa9169205a275/benchconnect/benchconnect/_start.py#L36


Fix patch:
diff --git a/benchadapt/python/README.md b/benchadapt/python/README.md
index c1adbd77..295612c0 100644
--- a/benchadapt/python/README.md
+++ b/benchadapt/python/README.md
@@ -4,18 +4,18 @@ A small python package with utilities for getting benchmark results into a Conbe
 
 ## Useful components in this package
 
-### `BenchmarkResult` and `BenchmarkRun` dataclasses
+### `BenchmarkResult` dataclass
 
-The `BenchmarkResult` and `BenchmarkRun` dataclasses are designed to make it easy to
-populate JSON payloads to post to a Conbench server. Their structure corresponds to the
-corresponding POST endpoint; they each have a `.to_publishable_dict()` method that
+The `BenchmarkResult` dataclass is designed to make it easy to
+populate JSON payloads to post to a Conbench server. The structure corresponds to the
+corresponding POST endpoint; it has a `.to_publishable_dict()` method that
 produces a dict to post.
 
-Regardless of how you are using them, the docstrings of these two objects will be useful
+Regardless of how you are using it, the docstrings of this object will be useful
 as you try to assemble your results to get them in Conbench. All fields are documented,
 as are interactions between them and what you likely need to specify.
 
-The objects try to help you fill in your payloads correctly, including some defaults,
+The object tries to help you fill in your payloads correctly, including some defaults,
 like populating `machine_info` with metadata on the current machine. If you are running
 on a cluster instead, you will need to populate `cluster_info` yourself, and
 `machine_info` will remain empty.
diff --git a/benchadapt/python/benchadapt/__init__.py b/benchadapt/python/benchadapt/__init__.py
index 4afdd50d..62d2a207 100644
--- a/benchadapt/python/benchadapt/__init__.py
+++ b/benchadapt/python/benchadapt/__init__.py
@@ -1,6 +1,5 @@
 from . import adapters
 from ._version import __version__
 from .result import BenchmarkResult
-from .run import BenchmarkRun
 
-__all__ = ["__version__", "adapters", "BenchmarkResult", "BenchmarkRun"]
+__all__ = ["__version__", "adapters", "BenchmarkResult"]
diff --git a/benchadapt/python/benchadapt/run.py b/benchadapt/python/benchadapt/run.py
deleted file mode 100644
index e359ec21..00000000
--- a/benchadapt/python/benchadapt/run.py
+++ /dev/null
@@ -1,150 +0,0 @@
-import uuid
-import warnings
-from dataclasses import asdict, dataclass, field
-from typing import Any, Dict
-
-from . import _machine_info
-
-
-@dataclass
-class BenchmarkRun:
-    """
-    A dataclass for containing metadata on a run of benchmarks.
-
-    Attributes
-    ----------
-    name : str
-        Name for the run. Current convention is ``f"{reason}: {github['commit']}"``.
-        If missing and ``github["commit"]`` exists, ``name`` will be populated
-        according to that pattern (even if ``reason`` is ``None``); otherwise it will
-        remain ``None``. Users should not set this manually unless they want to identify
-        runs in some other fashion. Benchmark name should be specified in ``tags["name"]``.
-    id : str
-        ID for the run. A hex UUID will be generated if not supplied.
-    reason : str
-        Reason for run (e.g. commit, PR, merge, nightly). Should be of low cardinality.
-    info : Dict[str, Any]
-        A schema-less dict of metadata about the run
-    machine_info : Dict[str, Any]
-        For benchmarks run on a single node, information about the machine, e.g. OS,
-        architecture, etc. Auto-populated if ``cluster_info`` not set. If host name
-        should not be detected with ``platform.node()`` (e.g. because a consistent
-        name is needed for CI or cloud runners), it can be overridden with the
-        ``CONBENCH_MACHINE_INFO_NAME`` environment variable.
-    cluster_info : Dict[str, Any]
-        For benchmarks run on a cluster, information about the cluster
-    github : Dict[str, Any]
-        See BenchmarkResult.
-    finished_timestamp : str
-        Timestamp the run finished, in ISO format, UTC or with timezone offset
-    error_type: str
-        Run's error type. Possible values: ``"none"``, ``"catastrophic"``, ``"partial"``.
-        ``"none"``: all attempted benchmarks are good.
-        ``"catastrophic"``: no benchmarks completed successfully.
-        ``"partial"``: some benchmarks completed, some failed.
-    error_info : Dict[str, Any]
-        A dict containing information about errors raised when running the set of benchmark.
-        Any schema is acceptable, but may contain stderr, a traceback, etc.
-
-    Notes
-    -----
-    Fields one of which must be supplied, the other of which will not be posted to conbench:
-
-    - ``machine_info`` (generated by default) or ``cluster_info``
-
-    Fields without comprehensive defaults which should be specified directly:
-
-    - ``reason``
-
-    Fields with defaults you may want to override on instantiation:
-
-    - ``name``
-    - ``id``
-    - ``info``
-    - ``machine_info`` if not run on the current machine
-    - ``cluster_info`` if run on a cluster
-    - ``github``
-    - ``finished_timestamp`` if run is now complete
-    - ``error_type`` (if applicable)
-    - ``error_info`` (if applicable)
-    """
-
-    name: str = None
-    id: str = field(default_factory=lambda: uuid.uuid4().hex)
-    reason: str = None
-    info: Dict[str, Any] = field(default_factory=dict)
-    machine_info: Dict[str, Any] = field(default_factory=_machine_info.machine_info)
-    cluster_info: Dict[str, Any] = None
-    github: Dict[str, str] = field(
-        default_factory=_machine_info.gh_commit_info_from_env
-    )
-    finished_timestamp: str = None
-    error_type: str = None
-    error_info: Dict[str, Any] = None
-
-    def __post_init__(self) -> None:
-        self._maybe_set_name()
-
-    def _maybe_set_name(self) -> None:
-        """
-        Set a default value for `name` if not populated and `github["commit"]` is.
-        Uses `reason`, but does not check if it's set, so may produce
-        `None: <commit hash>`. Since reason and commit are required by the API, this
-        should in most situations produce a reasonably useful `name`.
-        """
-        if not self.name:
-            if self.github.get("commit"):
-                self.name = f"{self.reason}: {self.github['commit']}"
-
-    @property
-    def _github_property(self):
-        return self._github_cache
-
-    @_github_property.setter
-    def _github_property(self, value: Dict[str, str]):
-        # Better: schema validation
-        if not isinstance(value, dict):
-            raise TypeError(f"unexpected type for `github` property: {value}")
-
-        if "repository" not in value:
-            raise ValueError(f"missing `repository` key in `github` property: {value}")
-
-        self._github_cache = value
-        self._maybe_set_name()
-
-    @property
-    def _cluster_info_property(self) -> Dict[str, Any]:
-        return self._cluster_info_cache
-
-    @_cluster_info_property.setter
-    def _cluster_info_property(self, value: Dict[str, Any]) -> None:
-        if value:
-            self.machine_info = None
-        self._cluster_info_cache = value
-
-    def to_publishable_dict(self):
-        """Returns a dict suitable for sending to conbench"""
-        res_dict = asdict(self)
-
-        if bool(res_dict.get("machine_info")) != bool(not res_dict["cluster_info"]):
-            warnings.warn(
-                "Run not publishable! `machine_info` xor `cluster_info` must be specified"
-            )
-
-        for attr in [
-            "name",
-            "machine_info",
-            "cluster_info",
-            "finished_timestamp",
-            "error_type",
-            "error_info",
-        ]:
-            if not res_dict[attr]:
-                res_dict.pop(attr)
-
-        return res_dict
-
-
-# Ugly, but per https://stackoverflow.com/a/61480946 lets us keep defaults and order
-BenchmarkRun.cluster_info = BenchmarkRun._cluster_info_property
-BenchmarkRun.github = BenchmarkRun._github_property


Test patch:
diff --git a/benchadapt/python/tests/test_result.py b/benchadapt/python/tests/test_result.py
index 34c6dbbc..161faa03 100644
--- a/benchadapt/python/tests/test_result.py
+++ b/benchadapt/python/tests/test_result.py
@@ -106,7 +106,7 @@ def test_commit_info_without_hash(self, monkeypatch):
         monkeypatch.setenv("CONBENCH_PROJECT_REPOSITORY", "http://localhost")
         monkeypatch.delenv("CONBENCH_PROJECT_PR_NUMBER", raising=False)
         monkeypatch.delenv("CONBENCH_PROJECT_COMMIT", raising=False)
-        res = BenchmarkResult()
+        res = BenchmarkResult(stats={"data": [47.0]})
         d = res.to_publishable_dict()
         assert d["github"] == {"repository": "http://localhost"}
 
diff --git a/benchadapt/python/tests/test_run.py b/benchadapt/python/tests/test_run.py
deleted file mode 100644
index 39f96a47..00000000
--- a/benchadapt/python/tests/test_run.py
+++ /dev/null
@@ -1,90 +0,0 @@
-import pytest
-
-from benchadapt import BenchmarkRun
-
-run_json = {
-    "name": "very-real-benchmark",
-    "id": "ezf69672dc3741259aac97650414a18c",
-    "reason": "test",
-    "finished_timestamp": "2202-09-16T15:42:27.527948+00:00",
-    "error_info": {"stack_trace": ["nothing", "really", "went", "wrong"]},
-    "error_type": "none really",
-    "info": {},
-    "machine_info": {
-        "name": "beepboop.local",
-        "os_name": "macOS",
-        "os_version": "12.6",
-        "architecture_name": "arm64",
-        "kernel_name": "21.6.0",
-        "memory_bytes": "17179869184",
-        "cpu_model_name": "Apple M3 Pro",
-        "cpu_core_count": "100",
-        "cpu_thread_count": "100",
-        "cpu_l1d_cache_bytes": "655360",
-        "cpu_l1i_cache_bytes": "1310720",
-        "cpu_l2_cache_bytes": "41943040",
-        "cpu_l3_cache_bytes": "0",
-        "cpu_frequency_max_hz": "0",
-        "gpu_count": "0",
-        "gpu_product_names": [],
-    },
-    "github": {
-        "commit": "2z8c9c49a5dc4a179243268e4bb6daa5",
-        "repository": "git@github.com:conchair/conchair",
-        "pr_number": "47",
-    },
-}
-
-
-class TestBenchmarkRun:
-    def test_roundtrip(self):
-        run = BenchmarkRun(**run_json)
-        assert run_json == run.to_publishable_dict()
-
-    def test_warns_machine_cluster(self):
-        with pytest.warns(UserWarning, match="Run not publishable!"):
-            BenchmarkRun(
-                machine_info={}, cluster_info={}, github=run_json["github"]
-            ).to_publishable_dict()
-
-        with pytest.warns(UserWarning, match="Run not publishable!"):
-            BenchmarkRun(
-                machine_info=None, cluster_info=None, github=run_json["github"]
-            ).to_publishable_dict()
-
-    def test_commit_detection(self, monkeypatch):
-        monkeypatch.setenv(
-            "CONBENCH_PROJECT_REPOSITORY", run_json["github"]["repository"]
-        )
-        monkeypatch.setenv(
-            "CONBENCH_PROJECT_PR_NUMBER", run_json["github"]["pr_number"]
-        )
-        monkeypatch.setenv("CONBENCH_PROJECT_COMMIT", run_json["github"]["commit"])
-        assert BenchmarkRun().github == run_json["github"]
-
-    def test_host_detection(self, monkeypatch):
-        machine_info_name = "fake-computer-name"
-        monkeypatch.setenv("CONBENCH_MACHINE_INFO_NAME", machine_info_name)
-
-        run = BenchmarkRun(github=run_json["github"])
-
-        assert run.machine_info["name"] == machine_info_name
-        assert run.machine_info["name"] != run_json["machine_info"]["name"]
-
-    def test_name_defaulting(self, monkeypatch):
-        monkeypatch.setenv("CONBENCH_PROJECT_REPOSITORY", "http://localhost")
-        monkeypatch.delenv("CONBENCH_PROJECT_PR_NUMBER", raising=False)
-        monkeypatch.delenv("CONBENCH_PROJECT_COMMIT", raising=False)
-        run = BenchmarkRun(reason=run_json["reason"])
-        assert run.github == {"repository": "http://localhost"}
-        assert run.name is None
-        run.github = run_json["github"]
-        assert run.name == f"{run_json['reason']}: {run_json['github']['commit']}"
-
-    def test_commit_info_without_hash(self, monkeypatch):
-        monkeypatch.setenv("CONBENCH_PROJECT_REPOSITORY", "http://localhost")
-        monkeypatch.delenv("CONBENCH_PROJECT_PR_NUMBER", raising=False)
-        monkeypatch.delenv("CONBENCH_PROJECT_COMMIT", raising=False)
-        run = BenchmarkRun()
-        d = run.to_publishable_dict()
-        assert d["github"] == {"repository": "http://localhost"}