Unify history and distribution API (#124)

conbench · Jul 26, 2021 · fec3d51 · fec3d51
1 parent 20d57f6
commit fec3d51
Show file tree

Hide file tree

Showing 10 changed files with 127 additions and 200 deletions.
diff --git a/conbench/api/__init__.py b/conbench/api/__init__.py
@@ -11,7 +11,7 @@
 from .commits import *  # noqa
 from .compare import *  # noqa
 from .contexts import *  # noqa
-from .distributions import *  # noqa
+from .distribution import *  # noqa
 from .history import *  # noqa
 from .index import *  # noqa
 from .machines import *  # noqa

diff --git a/conbench/api/_docs.py b/conbench/api/_docs.py
@@ -70,7 +70,6 @@ def _201_created(example, schema=None):
 spec.components.response("CompareList", _200_ok(ex.COMPARE_LIST))
 spec.components.response("ContextEntity", _200_ok(ex.CONTEXT_ENTITY))
 spec.components.response("ContextList", _200_ok([ex.CONTEXT_ENTITY]))
-spec.components.response("DistributionEntity", _200_ok(ex.DISTRIBUTION_ENTITY))
 spec.components.response("DistributionList", _200_ok([ex.DISTRIBUTION_ENTITY]))
 spec.components.response("HistoryList", _200_ok([ex.HISTORY_ENTITY]))
 spec.components.response("MachineEntity", _200_ok(ex.MACHINE_ENTITY))
@@ -90,7 +89,7 @@ def _201_created(example, schema=None):
     {"name": "Commits", "description": "Benchmarked commits"},
     {"name": "Comparisons", "description": "Benchmark comparisons"},
     {"name": "Contexts", "description": "Benchmark contexts"},
-    {"name": "Distributions", "description": "Benchmark distributions"},
+    {"name": "Distribution", "description": "Benchmark distribution"},
     {"name": "History", "description": "Benchmark history"},
     {"name": "Machines", "description": "Benchmark machines"},
     {"name": "Runs", "description": "Benchmark runs"},

diff --git a/conbench/api/_examples.py b/conbench/api/_examples.py
@@ -226,51 +226,39 @@ def _api_context_entity(context_id, links=True):
 
 def _api_distribution_entity(
     distribution_id,
-    sha,
     case_id,
     context_id,
-    machine_hash,
-    observations,
-    links=True,
 ):
     result = {
         "id": distribution_id,
-        "sha": sha,
         "case_id": case_id,
         "context_id": context_id,
-        "machine_hash": machine_hash,
+        "machine_hash": "diana-2-4-17179869184",
         "unit": "s",
         "mean_mean": "0.036369",
+        "mean_sd": "0.000000",
+        "repository": "https://github.com/apache/arrow",
+        "sha": "02addad336ba19a654f9c857ede546331be7b631",
         "first_timestamp": "2021-02-25T01:02:51",
         "last_timestamp": "2021-02-25T01:02:51",
-        "observations": observations,
-        "repository": "https://github.com/apache/arrow",
-        "links": {
-            "list": "http://localhost/api/distributions/",
-            "self": "http://localhost/api/distributions/%s/" % distribution_id,
-        },
     }
-    if not links:
-        result.pop("links", None)
     return result
 
 
 def _api_history_entity(benchmark_id, case_id, context_id):
-    return [
-        {
-            "benchmark_id": benchmark_id,
-            "case_id": case_id,
-            "context_id": context_id,
-            "machine_hash": "diana-2-4-17179869184",
-            "mean": "0.036369",
-            "message": "ARROW-11771: [Developer][Archery] Move benchmark tests (so CI runs them)",
-            "repository": "https://github.com/apache/arrow",
-            "run_name": "commit: 02addad336ba19a654f9c857ede546331be7b631",
-            "sha": "02addad336ba19a654f9c857ede546331be7b631",
-            "timestamp": "2021-02-25T01:02:51",
-            "unit": "s",
-        },
-    ]
+    return {
+        "benchmark_id": benchmark_id,
+        "case_id": case_id,
+        "context_id": context_id,
+        "machine_hash": "diana-2-4-17179869184",
+        "unit": "s",
+        "mean": "0.036369",
+        "repository": "https://github.com/apache/arrow",
+        "sha": "02addad336ba19a654f9c857ede546331be7b631",
+        "timestamp": "2021-02-25T01:02:51",
+        "message": "ARROW-11771: [Developer][Archery] Move benchmark tests (so CI runs them)",
+        "run_name": "commit: 02addad336ba19a654f9c857ede546331be7b631",
+    }
 
 
 def _api_machine_entity(machine_id, links=True):
@@ -374,11 +362,8 @@ def _api_run_entity(run_id, commit_id, machine_id, now, baseline_id):
 CONTEXT_ENTITY = _api_context_entity("some-context-uuid-1")
 DISTRIBUTION_ENTITY = _api_distribution_entity(
     "some-distribution-uuid-1",
-    "some-sha-1",
     "some-case-uuid-1",
     "some-context-uuid-1",
-    "some-machine-hash-1",
-    33,
 )
 HISTORY_ENTITY = _api_history_entity(
     "some-benchmark-uuid-1",
@@ -432,7 +417,6 @@ def _api_run_entity(run_id, commit_id, machine_id, now, baseline_id):
         "benchmarks": "http://localhost/api/benchmarks/",
         "commits": "http://localhost/api/commits/",
         "contexts": "http://localhost/api/contexts/",
-        "distributions": "http://localhost/api/distributions/",
         "docs": "http://localhost/api/docs.json",
         "login": "http://localhost/api/login/",
         "logout": "http://localhost/api/logout/",

diff --git a/conbench/api/distribution.py b/conbench/api/distribution.py
@@ -0,0 +1,50 @@
+from ..api import rule
+from ..api._endpoint import ApiEndpoint
+from ..entities._entity import NotFound
+from ..entities.distribution import get_distribution_history, DistributionSerializer
+from ..entities.summary import Summary
+
+
+class DistributionEntityAPI(ApiEndpoint):
+    serializer = DistributionSerializer()
+
+    def _get(self, benchmark_id):
+        try:
+            summary = Summary.one(id=benchmark_id)
+        except NotFound:
+            self.abort_404_not_found()
+        return get_distribution_history(
+            summary.run.commit.repository,
+            summary.run.commit.sha,
+            summary.case_id,
+            summary.context_id,
+            summary.run.machine.hash,
+        )
+
+    def get(self, benchmark_id):
+        """
+        ---
+        description: Get benchmark distribution history.
+        responses:
+            "200": "DistributionList"
+            "401": "401"
+            "404": "404"
+        parameters:
+          - name: benchmark_id
+            in: path
+            schema:
+                type: string
+        tags:
+          - Distribution
+        """
+        distribution = self._get(benchmark_id)
+        return self.serializer.many.dump(distribution)
+
+
+distribution_entity_view = DistributionEntityAPI.as_view("distribution")
+
+rule(
+    "/distribution/<benchmark_id>/",
+    view_func=distribution_entity_view,
+    methods=["GET"],
+)
diff --git a/conbench/api/distributions.py b/conbench/api/distributions.py
diff --git a/conbench/api/index.py b/conbench/api/index.py
@@ -23,7 +23,6 @@ def dump(self):
                 "benchmarks": f.url_for("api.benchmarks", _external=True),
                 "commits": f.url_for("api.commits", _external=True),
                 "contexts": f.url_for("api.contexts", _external=True),
-                "distributions": f.url_for("api.distributions", _external=True),
                 "docs": f.url_for("api.docs", _external=True),
                 "login": f.url_for("api.login", _external=True),
                 "logout": f.url_for("api.logout", _external=True),

diff --git a/conbench/entities/distribution.py b/conbench/entities/distribution.py
@@ -1,4 +1,3 @@
-import flask as f
 import sqlalchemy as s
 from sqlalchemy import func
 from sqlalchemy import CheckConstraint as check
@@ -61,6 +60,7 @@ class _Serializer(EntitySerializer):
     decimal_fmt = "{:.6f}"
 
     def _dump(self, distribution):
+        standard_deviation = distribution.mean_sd if distribution.mean_sd else 0
         result = {
             "id": distribution.id,
             "sha": distribution.sha,
@@ -70,15 +70,9 @@ def _dump(self, distribution):
             "machine_hash": distribution.machine_hash,
             "unit": distribution.unit,
             "mean_mean": self.decimal_fmt.format(distribution.mean_mean),
+            "mean_sd": self.decimal_fmt.format(standard_deviation),
             "first_timestamp": distribution.first_timestamp.isoformat(),
             "last_timestamp": distribution.last_timestamp.isoformat(),
-            "observations": distribution.observations,
-            "links": {
-                "list": f.url_for("api.distributions", _external=True),
-                "self": f.url_for(
-                    "api.distribution", distribution_id=distribution.id, _external=True
-                ),
-            },
         }
         return result
 
@@ -88,6 +82,33 @@ class DistributionSerializer:
     many = _Serializer(many=True)
 
 
+def get_distribution_history(repository, sha, case_id, context_id, machine_hash):
+    return (
+        Session.query(
+            Distribution.id,
+            Distribution.repository,
+            Distribution.sha,
+            Distribution.case_id,
+            Distribution.context_id,
+            Distribution.machine_hash,
+            Distribution.unit,
+            Distribution.mean_mean,
+            Distribution.mean_sd,
+            Distribution.first_timestamp,
+            Distribution.last_timestamp,
+        )
+        .filter(
+            Distribution.repository == repository,
+            Distribution.sha == sha,
+            Distribution.case_id == case_id,
+            Distribution.context_id == context_id,
+            Distribution.machine_hash == machine_hash,
+        )
+        .order_by(Distribution.first_timestamp.asc())
+        .all()
+    )
+
+
 def get_commit_index(repository):
     ordered = (
         Session.query(Commit.id, Commit.sha, Commit.timestamp)