Repository: conbench/conbench (language: python)

Issue:
benchconnect: minimal CLI for posting to conbench
This is an interpretation of https://github.com/conbench/conbench/issues/368#issuecomment-1297215937 that builds a minimal CLI that currently can post benchmark results from an inline JSON dict, a JSON file, or a dir of JSON files.

This is deliberately minimal to elicit comment about

1. what about the behavior/interface do we like?
2. what do we want to change?
3. what functionality is missing that we want/need?

Obviously the other side that calls this needs to exist, too! But that would live elsewhere.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml
index 2b05da194..1eda3a7af 100755
--- a/.github/workflows/actions.yml
+++ b/.github/workflows/actions.yml
@@ -7,7 +7,7 @@ on:
   pull_request:
 
 env:
-  PYTHON_VERSION: '3.8'
+  PYTHON_VERSION: "3.8"
 
 jobs:
   build:
@@ -36,11 +36,12 @@ jobs:
           src: .
       - name: Install dependencies
         run: |
-          pip install \
+          pip install -e benchadapt/python --no-deps && \
+            pip install \
             -U --upgrade-strategy eager \
             -e .[dev] \
-            -e benchadapt/python \
-            -e benchrun/python
+              -e benchclients/python \
+              -e benchrun/python
       - name: Migrations
         run: |
           alembic upgrade head
@@ -66,6 +67,9 @@ jobs:
           coveralls --service=github
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - name: Test benchclients
+        run: |
+          pytest -vv benchclients/python/tests
       - name: Test benchadapt
         run: |
           pytest -vv benchadapt/python/tests
diff --git a/benchadapt/python/benchadapt/client.py b/benchadapt/python/benchadapt/client.py
index 2e6d88e80..b28902a0e 100644
--- a/benchadapt/python/benchadapt/client.py
+++ b/benchadapt/python/benchadapt/client.py
@@ -1,104 +1,4 @@
-# All copied from https://github.com/conbench/benchalerts/blob/main/benchalerts/clients.py
-# TODO: extract client into a package and have both packages use it as a dep (or otherwise
-# use the same code somehow)
+import benchclients
 
-import abc
-import os
-from json import dumps
-from typing import Optional
-
-import requests
-from requests.adapters import HTTPAdapter
-from urllib3.util.retry import Retry
-
-from .log import fatal_and_log, log
-
-
-class _BaseClient(abc.ABC):
-    """A client to interact with an API.
-    Parameters
-    ----------
-    adapter
-        A requests adapter to mount to the requests session. If not given, one will be
-        created with a backoff retry strategy.
-    """
-
-    base_url: str
-    timeout_s = 10
-
-    def __init__(self, adapter: Optional[HTTPAdapter]):
-        if not adapter:
-            retry_strategy = Retry(
-                total=5,
-                status_forcelist=frozenset((429, 502, 503, 504)),
-                backoff_factor=4,  # will retry in 2, 4, 8, 16, 32 seconds
-            )
-            adapter = HTTPAdapter(max_retries=retry_strategy)
-
-        self.session = requests.Session()
-        self.session.mount("https://", adapter)
-
-    def get(self, path: str) -> dict:
-        url = self.base_url + path
-        log.debug(f"GET {url}")
-        res = self.session.get(url=url, timeout=self.timeout_s)
-        self._maybe_raise(res=res)
-
-        return res.json()
-
-    def post(self, path: str, json: dict) -> Optional[dict]:
-        url = self.base_url + path
-
-        # log.debug(f"POST {url} {dumps(json)}")
-        print(f"POST {url} {dumps(json)}")
-
-        res = self.session.post(url=url, json=json, timeout=self.timeout_s)
-        self._maybe_raise(res=res)
-
-        if res.content:
-            return res.json()
-
-    @staticmethod
-    def _maybe_raise(res: requests.Response):
-        try:
-            res.raise_for_status()
-        except requests.HTTPError as e:
-            try:
-                res_content = e.response.content.decode()
-            except AttributeError:
-                res_content = e.response.content
-            log.error(f"Response content: {res_content}")
-            raise
-
-
-class ConbenchClient(_BaseClient):
-    """A client to interact with a Conbench server.
-    Parameters
-    ----------
-    adapter
-        A requests adapter to mount to the requests session. If not given, one will be
-        created with a backoff retry strategy.
-    Environment variables
-    ---------------------
-    CONBENCH_URL
-        The URL of the Conbench server. Required.
-    CONBENCH_EMAIL
-        The email to use for Conbench login. Only required if the server is private.
-    CONBENCH_PASSWORD
-        The password to use for Conbench login. Only required if the server is private.
-    """
-
-    def __init__(self, adapter: Optional[HTTPAdapter] = None):
-        url = os.getenv("CONBENCH_URL")
-        if not url:
-            fatal_and_log("Environment variable CONBENCH_URL not found")
-
-        super().__init__(adapter=adapter)
-        self.base_url = url + "/api"
-
-        login_creds = {
-            "email": os.getenv("CONBENCH_EMAIL"),
-            "password": os.getenv("CONBENCH_PASSWORD"),
-        }
-        if login_creds["email"] and login_creds["password"]:
-            self.post("/login/", json=login_creds)
+# reexport
+ConbenchClient = benchclients.ConbenchClient
diff --git a/benchadapt/python/requirements.txt b/benchadapt/python/requirements.txt
index 804abb1bf..bf6213eae 100644
--- a/benchadapt/python/requirements.txt
+++ b/benchadapt/python/requirements.txt
@@ -1,2 +1,1 @@
-requests
-urllib3
+benchclients@git+https://github.com/conbench/conbench.git@main#subdirectory=benchclients/python
diff --git a/benchclients/README.md b/benchclients/README.md
new file mode 100644
index 000000000..9e9b65c44
--- /dev/null
+++ b/benchclients/README.md
@@ -0,0 +1,3 @@
+# benchclients
+
+Packages with clients for working with APIs in the Conbench ecosystem
diff --git a/benchclients/python/README.md b/benchclients/python/README.md
new file mode 100644
index 000000000..14a2fbd3f
--- /dev/null
+++ b/benchclients/python/README.md
@@ -0,0 +1,3 @@
+# benchclients
+
+A lightweight Python package with clients for working with APIs in the Conbench ecosystem
diff --git a/benchclients/python/benchclients/__init__.py b/benchclients/python/benchclients/__init__.py
new file mode 100644
index 000000000..dd4a1c552
--- /dev/null
+++ b/benchclients/python/benchclients/__init__.py
@@ -0,0 +1,6 @@
+from ._version import __version__
+from .base import BaseClient
+from .conbench import ConbenchClient
+from .logging import log
+
+__all__ = ["__version__", "BaseClient", "ConbenchClient", "log"]
diff --git a/benchclients/python/benchclients/_version.py b/benchclients/python/benchclients/_version.py
new file mode 100644
index 000000000..f102a9cad
--- /dev/null
+++ b/benchclients/python/benchclients/_version.py
@@ -0,0 +1,1 @@
+__version__ = "0.0.1"
diff --git a/benchclients/python/benchclients/base.py b/benchclients/python/benchclients/base.py
new file mode 100644
index 000000000..32e67d607
--- /dev/null
+++ b/benchclients/python/benchclients/base.py
@@ -0,0 +1,75 @@
+import abc
+from json import dumps
+from typing import Optional
+
+import requests
+from requests.adapters import HTTPAdapter
+from urllib3.util.retry import Retry
+
+from .logging import log
+
+
+class BaseClient(abc.ABC):
+    """A client to interact with an API.
+
+    Parameters
+    ----------
+    adapter
+        A requests adapter to mount to the requests session. If not given, one will be
+        created with a backoff retry strategy.
+    """
+
+    base_url: str
+    timeout_s = 10
+
+    def __init__(self, adapter: Optional[HTTPAdapter]):
+        if not adapter:
+            retry_strategy = Retry(
+                total=5,
+                status_forcelist=frozenset((429, 502, 503, 504)),
+                backoff_factor=4,  # will retry in 2, 4, 8, 16, 32 seconds
+            )
+            adapter = HTTPAdapter(max_retries=retry_strategy)
+
+        self.session = requests.Session()
+        self.session.mount("https://", adapter)
+
+    def get(self, path: str) -> dict:
+        url = self.base_url + path
+        log.debug(f"GET {url}")
+        res = self.session.get(url=url, timeout=self.timeout_s)
+        self._maybe_raise(res=res)
+
+        return res.json()
+
+    def post(self, path: str, json: dict) -> Optional[dict]:
+        url = self.base_url + path
+
+        log.debug(f"POST {url} {dumps(json)}")
+
+        res = self.session.post(url=url, json=json, timeout=self.timeout_s)
+        self._maybe_raise(res=res)
+
+        if res.content:
+            return res.json()
+
+    def put(self, path: str, json: dict) -> Optional[dict]:
+        url = self.base_url + path
+        log.debug(f"PUT {url} {dumps(json)}")
+        res = self.session.put(url=url, json=json, timeout=self.timeout_s)
+        self._maybe_raise(res=res)
+
+        if res.content:
+            return res.json()
+
+    @staticmethod
+    def _maybe_raise(res: requests.Response):
+        try:
+            res.raise_for_status()
+        except requests.HTTPError as e:
+            try:
+                res_content = e.response.content.decode()
+            except AttributeError:
+                res_content = e.response.content
+            log.error(f"Response content: {res_content}")
+            raise
diff --git a/benchclients/python/benchclients/conbench.py b/benchclients/python/benchclients/conbench.py
new file mode 100644
index 000000000..01f288418
--- /dev/null
+++ b/benchclients/python/benchclients/conbench.py
@@ -0,0 +1,41 @@
+import os
+from typing import Optional
+
+from requests.adapters import HTTPAdapter
+
+from .base import BaseClient
+from .logging import fatal_and_log
+
+
+class ConbenchClient(BaseClient):
+    """A client to interact with a Conbench server.
+
+    Parameters
+    ----------
+    adapter
+        A requests adapter to mount to the requests session. If not given, one will be
+        created with a backoff retry strategy.
+    Environment variables
+    ---------------------
+    CONBENCH_URL
+        The URL of the Conbench server. Required.
+    CONBENCH_EMAIL
+        The email to use for Conbench login. Only required if the server is private.
+    CONBENCH_PASSWORD
+        The password to use for Conbench login. Only required if the server is private.
+    """
+
+    def __init__(self, adapter: Optional[HTTPAdapter] = None):
+        url = os.getenv("CONBENCH_URL")
+        if not url:
+            fatal_and_log("Environment variable CONBENCH_URL not found")
+
+        super().__init__(adapter=adapter)
+        self.base_url = url + "/api"
+
+        login_creds = {
+            "email": os.getenv("CONBENCH_EMAIL"),
+            "password": os.getenv("CONBENCH_PASSWORD"),
+        }
+        if login_creds["email"] and login_creds["password"]:
+            self.post("/login/", json=login_creds)
diff --git a/benchclients/python/benchclients/logging.py b/benchclients/python/benchclients/logging.py
new file mode 100644
index 000000000..97db9f97b
--- /dev/null
+++ b/benchclients/python/benchclients/logging.py
@@ -0,0 +1,12 @@
+import logging
+
+logging.basicConfig(
+    format="%(levelname)s [%(asctime)s] %(message)s", level=logging.INFO
+)
+log = logging.getLogger(__name__)
+
+
+def fatal_and_log(msg: str, etype: BaseException = ValueError):
+    """If an error occurs, log the message and raise an exception."""
+    log.error(msg)
+    raise etype(msg)
diff --git a/benchclients/python/requirements-dev.txt b/benchclients/python/requirements-dev.txt
new file mode 100644
index 000000000..e079f8a60
--- /dev/null
+++ b/benchclients/python/requirements-dev.txt
@@ -0,0 +1,1 @@
+pytest
diff --git a/benchclients/python/requirements.txt b/benchclients/python/requirements.txt
new file mode 100644
index 000000000..804abb1bf
--- /dev/null
+++ b/benchclients/python/requirements.txt
@@ -0,0 +1,2 @@
+requests
+urllib3
diff --git a/benchclients/python/setup.py b/benchclients/python/setup.py
new file mode 100644
index 000000000..055121caf
--- /dev/null
+++ b/benchclients/python/setup.py
@@ -0,0 +1,45 @@
+import pathlib
+from typing import List
+
+import setuptools
+
+repo_root = pathlib.Path(__file__).parent
+
+with open(repo_root / "README.md", "r") as readme:
+    long_description = readme.read()
+
+__version__ = ""
+with open(repo_root / "benchclients" / "_version.py", "r") as f:
+    exec(f.read())  # only overwrites the __version__ variable
+
+
+def read_requirements_file(filepath: pathlib.Path) -> List[str]:
+    """Parse a requirements.txt file into a list of package requirements"""
+    with open(filepath, "r") as f:
+        requirements = [
+            line.strip() for line in f if line.strip() and not line.startswith("#")
+        ]
+    return requirements
+
+
+base_requirements = read_requirements_file(repo_root / "requirements.txt")
+dev_requirements = read_requirements_file(repo_root / "requirements-dev.txt")
+
+setuptools.setup(
+    name="benchclients",
+    version=__version__,
+    description="Clients for the Conbench ecosystem",
+    long_description=long_description,
+    long_description_content_type="text/markdown",
+    packages=setuptools.find_packages(),
+    classifiers=[
+        "Programming Language :: Python :: 3.8",
+        "License :: OSI Approved :: Apache Software License",
+    ],
+    python_requires=">=3.8",
+    maintainer="Voltron Data",
+    maintainer_email="conbench@voltrondata.com",
+    url="https://github.com/conbench/conbench/tree/main/benchclients",
+    install_requires=base_requirements,
+    extras_require={"dev": dev_requirements},
+)
diff --git a/benchrun/python/requirements.txt b/benchrun/python/requirements.txt
index e69de29bb..5fe5c499e 100644
--- a/benchrun/python/requirements.txt
+++ b/benchrun/python/requirements.txt
@@ -0,0 +1,1 @@
+benchadapt@git+https://github.com/conbench/conbench.git@main#subdirectory=benchadapt/python


Test patch:
diff --git a/benchclients/python/tests/unit_tests/__init__.py b/benchclients/python/tests/unit_tests/__init__.py
new file mode 100644
index 000000000..65164120e
--- /dev/null
+++ b/benchclients/python/tests/unit_tests/__init__.py
@@ -0,0 +1,13 @@
+# Copyright (c) 2022, Voltron Data.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
diff --git a/benchclients/python/tests/unit_tests/conftest.py b/benchclients/python/tests/unit_tests/conftest.py
new file mode 100644
index 000000000..699e5dfee
--- /dev/null
+++ b/benchclients/python/tests/unit_tests/conftest.py
@@ -0,0 +1,29 @@
+# Copyright (c) 2022, Voltron Data.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import pytest
+
+
+@pytest.fixture
+def conbench_env(monkeypatch: pytest.MonkeyPatch):
+    monkeypatch.setenv("CONBENCH_URL", "https://conbench.biz")
+    monkeypatch.setenv("CONBENCH_EMAIL", "email")
+    monkeypatch.setenv("CONBENCH_PASSWORD", "password")
+
+
+@pytest.fixture
+def missing_conbench_env(monkeypatch: pytest.MonkeyPatch):
+    monkeypatch.delenv("CONBENCH_URL", raising=False)
+    monkeypatch.delenv("CONBENCH_EMAIL", raising=False)
+    monkeypatch.delenv("CONBENCH_PASSWORD", raising=False)
diff --git a/benchclients/python/tests/unit_tests/mocked_responses/GET_conbench_error_with_content.json b/benchclients/python/tests/unit_tests/mocked_responses/GET_conbench_error_with_content.json
new file mode 100644
index 000000000..d740f0a58
--- /dev/null
+++ b/benchclients/python/tests/unit_tests/mocked_responses/GET_conbench_error_with_content.json
@@ -0,0 +1,7 @@
+{
+    "status_code": 404,
+    "data": {
+        "code": 404,
+        "name": "Not Found"
+    }
+}
diff --git a/benchclients/python/tests/unit_tests/mocked_responses/GET_conbench_error_without_content.json b/benchclients/python/tests/unit_tests/mocked_responses/GET_conbench_error_without_content.json
new file mode 100644
index 000000000..0c9fef0f7
--- /dev/null
+++ b/benchclients/python/tests/unit_tests/mocked_responses/GET_conbench_error_without_content.json
@@ -0,0 +1,3 @@
+{
+    "status_code": 404
+}
diff --git a/benchclients/python/tests/unit_tests/mocked_responses/POST_conbench_login.json b/benchclients/python/tests/unit_tests/mocked_responses/POST_conbench_login.json
new file mode 100644
index 000000000..9cc9d9498
--- /dev/null
+++ b/benchclients/python/tests/unit_tests/mocked_responses/POST_conbench_login.json
@@ -0,0 +1,3 @@
+{
+    "status_code": 204
+}
diff --git a/benchclients/python/tests/unit_tests/mocks.py b/benchclients/python/tests/unit_tests/mocks.py
new file mode 100644
index 000000000..25e4cf3de
--- /dev/null
+++ b/benchclients/python/tests/unit_tests/mocks.py
@@ -0,0 +1,74 @@
+# Copyright (c) 2022, Voltron Data.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import json
+import pathlib
+
+import requests
+from requests.adapters import HTTPAdapter
+
+from benchclients import log
+
+response_dir = pathlib.Path(__file__).parent / "mocked_responses"
+
+
+class MockResponse(requests.Response):
+    def __init__(self, status_code, data=None):
+        super().__init__()
+        self.status_code = status_code
+        if data is not None:
+            self._content = str.encode(json.dumps(data))
+
+    @classmethod
+    def from_file(cls, file: pathlib.Path):
+        with open(file, "r") as f:
+            response: dict = json.load(f)
+        return cls(status_code=response["status_code"], data=response.get("data"))
+
+
+class MockAdapter(HTTPAdapter):
+    @staticmethod
+    def clean_base_url(url: str) -> str:
+        bases = {
+            "https://conbench.biz/api": "conbench",
+        }
+        for base_url, base_name in bases.items():
+            if url.startswith(base_url):
+                clean_path = url.split(base_url + "/")[1]
+                for char in ["/", "&", "?", "=", ".", "__", "__"]:
+                    clean_path = clean_path.replace(char, "_")
+                if clean_path.endswith("_"):
+                    clean_path = clean_path[:-1]
+                return base_name + "_" + clean_path if clean_path else base_name
+
+        raise Exception(f"No base URL was mocked for this request: {url}")
+
+    def send(self, *args, **kwargs):
+        req: requests.PreparedRequest = args[0]
+        log.info(f"Sent request {req}({req.__dict__}) with kwargs {kwargs}")
+
+        # to help with test_workflows.py, log the markdowns that were posted
+        if req.url.endswith("check-runs"):
+            body = json.loads(req.body)
+            log.info("Summary: " + body["output"]["summary"])
+            log.info("Details: " + str(body["output"].get("text")))
+
+        method = req.method
+        clean_url = self.clean_base_url(req.url)
+        response_path = response_dir / f"{method}_{clean_url}.json"
+
+        if not response_path.exists():
+            raise Exception(f"Mock response not found at {response_path}")
+
+        return MockResponse.from_file(response_path)
diff --git a/benchclients/python/tests/unit_tests/test_conbench.py b/benchclients/python/tests/unit_tests/test_conbench.py
new file mode 100644
index 000000000..3e7819d80
--- /dev/null
+++ b/benchclients/python/tests/unit_tests/test_conbench.py
@@ -0,0 +1,41 @@
+# Copyright (c) 2022, Voltron Data.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import pytest
+import requests
+from _pytest.logging import LogCaptureFixture
+
+from benchclients import ConbenchClient
+
+from .mocks import MockAdapter
+
+
+class TestConbenchClient:
+    @property
+    def cb(self):
+        return ConbenchClient(adapter=MockAdapter())
+
+    def test_conbench_fails_missing_env(self, missing_conbench_env):
+        with pytest.raises(ValueError, match="CONBENCH_URL"):
+            self.cb
+
+    @pytest.mark.parametrize("path", ["/error_with_content", "/error_without_content"])
+    def test_client_error_handling(self, conbench_env, path, caplog: LogCaptureFixture):
+        with pytest.raises(requests.HTTPError, match="404"):
+            self.cb.get(path)
+
+        if path == "/error_with_content":
+            assert 'Response content: {"code":' in caplog.text
+        else:
+            assert "Response content: None" in caplog.text
diff --git a/benchclients/python/tests/unit_tests/test_version.py b/benchclients/python/tests/unit_tests/test_version.py
new file mode 100644
index 000000000..26fdf0d8d
--- /dev/null
+++ b/benchclients/python/tests/unit_tests/test_version.py
@@ -0,0 +1,19 @@
+# Copyright (c) 2022, Voltron Data.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import benchclients
+
+
+def test_version_exists():
+    assert benchclients.__version__