You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by sb...@apache.org on 2026/09/03 18:04:30 UTC
(tooling-trusted-releases) branch sbp updated: Count check results in database queries for efficiency
This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch sbp
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
The following commit(s) were added to refs/heads/sbp by this push:
new 1bb79302 Count check results in database queries for efficiency
1bb79302 is described below
commit 1bb793027ac9b1a94ed29ea9b0dd77fd352552a1
Author: Sean B. Palmer <se...@miscoranda.com>
AuthorDate: Thu Sep 3 19:01:47 2026 +0100
Count check results in database queries for efficiency
---
atr/db/interaction.py | 54 +++++++++++++-
atr/get/checks.py | 33 ++++++---
atr/models/sql.py | 8 +++
atr/storage/datatypes.py | 2 +-
atr/storage/readers/releases.py | 41 ++++++-----
atr/storage/writers/release.py | 6 +-
atr/templates/check-selected-path-table.html | 2 +-
tests/unit/test_checker_stats.py | 9 ++-
tests/unit/test_checks_tally.py | 103 +++++++++++++++++++++++++++
tests/unit/test_get_checks_filestats.py | 19 +++--
10 files changed, 238 insertions(+), 39 deletions(-)
diff --git a/atr/db/interaction.py b/atr/db/interaction.py
index 59115982..bf8585df 100644
--- a/atr/db/interaction.py
+++ b/atr/db/interaction.py
@@ -19,7 +19,7 @@ import dataclasses
import datetime
import enum
from collections.abc import Sequence
-from typing import Final
+from typing import Final, NamedTuple
import asfquart.base as base
import packaging.version as version
@@ -78,6 +78,19 @@ class ReleasePolicyNotFoundError(RuntimeError):
pass
+class CheckCount(NamedTuple):
+ primary_rel_path: str | None
+ checker: str
+ status: sql.CheckResultStatus
+ member: bool
+ total: int
+
+
+class ChecksTally(NamedTuple):
+ counts: list[CheckCount]
+ results: list[sql.CheckResult]
+
+
class TrustedProjectPhase(enum.Enum):
COMPOSE = "compose"
VOTE = "vote"
@@ -340,6 +353,45 @@ async def checks_for(
)
+async def checks_tally_for(
+ release: sql.Release,
+ revision: safe.RevisionNumber | None = None,
+ *,
+ statuses: Sequence[sql.CheckResultStatus],
+ caller_data: db.Session | None = None,
+) -> ChecksTally:
+ if revision is None:
+ revision = release.safe_latest_revision_number
+ via = sql.validate_instrumented_attribute
+ async with db.ensure_session(caller_data) as data:
+ query = await _check_results_query(
+ data, release.safe_project_key, release.safe_version_key, revision, None, False, None
+ )
+ if query is None:
+ return ChecksTally([], [])
+ rows = query.query.subquery()
+ member = rows.c.member_rel_path.is_not(None).label("member")
+ counted = await data.execute(
+ sqlalchemy.select(
+ rows.c.primary_rel_path, rows.c.checker, rows.c.status, member, sqlalchemy.func.count()
+ ).group_by(rows.c.primary_rel_path, rows.c.checker, rows.c.status, member)
+ )
+ counts = [
+ CheckCount(path, checker, sql.CheckResultStatus(status), bool(flag), total)
+ for path, checker, status, flag, total in counted.all()
+ ]
+ results = await (
+ query.where(via(sql.CheckResult.status).in_(statuses))
+ .order_by(
+ via(sql.CheckResult.checker).asc(),
+ via(sql.CheckResult.created).desc(),
+ via(sql.CheckResult.id).desc(),
+ )
+ .all()
+ )
+ return ChecksTally(counts, list(results))
+
+
async def count_checks_for_revision_by_status(
status: sql.CheckResultStatus,
release: sql.Release,
diff --git a/atr/get/checks.py b/atr/get/checks.py
index e6fa86eb..40cbf0ad 100644
--- a/atr/get/checks.py
+++ b/atr/get/checks.py
@@ -231,18 +231,19 @@ async def _compute_stats(
return per_file, _file_stats_empty()
async with db.session() as data:
- check_results = await interaction.checks_for(release, caller_data=data)
+ tally = await interaction.checks_tally_for(
+ release, statuses=sql.CHECK_RESULT_IGNORABLE_STATUSES, caller_data=data
+ )
- for check_result in check_results:
- if not check_result.primary_rel_path:
- continue
+ for count in tally.counts:
+ stats = _file_stats_for(per_file, count.primary_rel_path)
+ if (stats is not None) and (count.status not in sql.CHECK_RESULT_IGNORABLE_STATUSES):
+ _file_stats_count_add(stats, count)
- file_path = safe.RelPath(check_result.primary_rel_path)
- stats = per_file.get(file_path)
- if stats is None:
- continue
-
- _file_stats_result_add(stats, check_result, match_ignore(check_result))
+ for check_result in tally.results:
+ stats = _file_stats_for(per_file, check_result.primary_rel_path)
+ if stats is not None:
+ _file_stats_result_add(stats, check_result, match_ignore(check_result))
totals = _file_stats_empty()
for stats in per_file.values():
@@ -271,6 +272,12 @@ def _error_count(counts: collections.Counter[sql.CheckResultStatus]) -> int:
)
+def _file_stats_count_add(stats: FileStats, count: interaction.CheckCount) -> None:
+ before, after = (stats.member_before, stats.member_after) if count.member else (stats.file_before, stats.file_after)
+ before[count.status] += count.total
+ after[count.status] += count.total
+
+
def _file_stats_empty() -> FileStats:
return FileStats(
file_before=collections.Counter[sql.CheckResultStatus](),
@@ -280,6 +287,12 @@ def _file_stats_empty() -> FileStats:
)
+def _file_stats_for(per_file: dict[safe.RelPath, FileStats], primary_rel_path: str | None) -> FileStats | None:
+ if not primary_rel_path:
+ return None
+ return per_file.get(safe.RelPath(primary_rel_path))
+
+
def _file_stats_result_add(stats: FileStats, check_result: sql.CheckResult, is_ignored: bool) -> None:
if check_result.member_rel_path is None:
before = stats.file_before
diff --git a/atr/models/sql.py b/atr/models/sql.py
index b3c0a9bc..550f61b8 100644
--- a/atr/models/sql.py
+++ b/atr/models/sql.py
@@ -116,6 +116,14 @@ class CheckResultStatusIgnore(enum.StrEnum):
return f"CheckResultStatusIgnore.{self.value.upper()}"
+CHECK_RESULT_IGNORABLE_STATUSES: Final[tuple[CheckResultStatus, ...]] = tuple(
+ CheckResultStatus(status.value) for status in CheckResultStatusIgnore
+)
+CHECK_RESULT_NON_NOTE_STATUSES: Final[tuple[CheckResultStatus, ...]] = tuple(
+ status for status in CheckResultStatus if (status != CheckResultStatus.NOTE)
+)
+
+
class DistributionPlatform(enum.Enum):
ARTIFACT_HUB = DistributionPlatformValue(
name="Artifact Hub",
diff --git a/atr/storage/datatypes.py b/atr/storage/datatypes.py
index b8645b8d..64c4c0d0 100644
--- a/atr/storage/datatypes.py
+++ b/atr/storage/datatypes.py
@@ -120,7 +120,7 @@ class PathInfo(schema.Strict):
ignored_concerns: list[sql.CheckResult] = schema.factory(list)
ignored_exceptions: list[sql.CheckResult] = schema.factory(list)
ignored_suggestions: list[sql.CheckResult] = schema.factory(list)
- notes: dict[safe.RelPath, list[sql.CheckResult]] = schema.factory(dict)
+ note_counts: dict[safe.RelPath, int] = schema.factory(dict)
release_level_blockers: list[sql.CheckResult] = schema.factory(list)
release_level_concerns: list[sql.CheckResult] = schema.factory(list)
release_level_exceptions: list[sql.CheckResult] = schema.factory(list)
diff --git a/atr/storage/readers/releases.py b/atr/storage/readers/releases.py
index e885d63d..340bac68 100644
--- a/atr/storage/readers/releases.py
+++ b/atr/storage/readers/releases.py
@@ -56,7 +56,14 @@ class GeneralPublic:
latest_revision_number = release.latest_revision_number
if latest_revision_number is None:
return None
- await self.__notes_suggestions_concerns(release, release.safe_latest_revision_number, info)
+ tally = await interaction.checks_tally_for(
+ release,
+ revision=release.safe_latest_revision_number,
+ statuses=sql.CHECK_RESULT_NON_NOTE_STATUSES,
+ caller_data=self.__data,
+ )
+ await self.__suggestions_concerns(release, info, tally.results)
+ self.__note_counts(info, tally.counts)
base_path = paths.release_directory(release)
revision_seq = int(str(release.safe_latest_revision_number))
db_classifications = await self.__data.release_file_classifications_at(release.key, revision_seq)
@@ -79,7 +86,7 @@ class GeneralPublic:
path, base_path=base_path, source_matcher=source_matcher, binary_matcher=binary_matcher
)
self.__pair_sboms(info, all_paths)
- self.__compute_checker_stats(info, all_paths)
+ self.__compute_checker_stats(info, all_paths, tally.counts)
return info
def __accumulate_results(
@@ -100,11 +107,16 @@ class GeneralPublic:
files_for_status = acc.files.setdefault(status, {})
files_for_status[path_str] = files_for_status.get(path_str, 0) + 1
- def __compute_checker_stats(self, info: datatypes.PathInfo, paths: list[safe.RelPath]) -> None:
+ def __compute_checker_stats(
+ self, info: datatypes.PathInfo, paths: list[safe.RelPath], counts: list[interaction.CheckCount]
+ ) -> None:
paths_set = set(paths)
+ path_strs = {str(path) for path in paths}
checker_data: dict[str, CheckerAccumulator] = {}
- self.__accumulate_results(info.notes, paths_set, checker_data)
+ for count in counts:
+ if (count.status == sql.CheckResultStatus.NOTE) and (count.primary_rel_path in path_strs):
+ checker_data.setdefault(count.checker, CheckerAccumulator()).counts[count.status] += count.total
self.__accumulate_results(info.suggestions, paths_set, checker_data)
self.__accumulate_results(info.concerns, paths_set, checker_data)
self.__accumulate_results(info.exceptions, paths_set, checker_data)
@@ -133,22 +145,18 @@ class GeneralPublic:
info.sbom_paths[path] = safe.RelPath(candidate)
break
- async def __notes_suggestions_concerns(
- self, release: sql.Release, latest_revision_number: safe.RevisionNumber, info: datatypes.PathInfo
+ async def __suggestions_concerns(
+ self, release: sql.Release, info: datatypes.PathInfo, results: list[sql.CheckResult]
) -> None:
match_ignore = await self.__read_as.checks.ignores_matcher(release.safe_project_key)
- attestable_checks = await interaction.checks_for(
- release, revision=latest_revision_number, caller_data=self.__data
- )
cs = datatypes.ChecksSubset(
- checks=attestable_checks,
+ checks=results,
info=info,
match_ignore=match_ignore,
)
# TODO: These get just the ones for the revision.
# It might be better to get all like we do in by_release_path, filter by hash, then filter by status
- await self.__notes(cs)
await self.__suggestions(cs)
await self.__concerns(cs)
await self.__exceptions(cs)
@@ -184,12 +192,11 @@ class GeneralPublic:
else:
cs.info.release_level_exceptions.append(exception)
- async def __notes(self, cs: datatypes.ChecksSubset) -> None:
- notes = [cr for cr in cs.checks if cr.status == sql.CheckResultStatus.NOTE]
- for note in notes:
- # Notes cannot be ignored
- if path := note.safe_primary_rel_path:
- cs.info.notes.setdefault(path, []).append(note)
+ def __note_counts(self, info: datatypes.PathInfo, counts: list[interaction.CheckCount]) -> None:
+ for count in counts:
+ if (count.status == sql.CheckResultStatus.NOTE) and count.primary_rel_path:
+ path = safe.RelPath(count.primary_rel_path)
+ info.note_counts[path] = info.note_counts.get(path, 0) + count.total
async def __suggestions(self, cs: datatypes.ChecksSubset) -> None:
suggestions = [cr for cr in cs.checks if cr.status == sql.CheckResultStatus.SUGGESTION]
diff --git a/atr/storage/writers/release.py b/atr/storage/writers/release.py
index 72f5eefd..afe0eb4d 100644
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -914,8 +914,10 @@ class CommitteeParticipant(FoundationCommitter):
async def __required_concern_groups(self, release: sql.Release) -> list[util.ConcernGroup]:
# Avoid a filesystem walk when the DB has no concern rows at all
- check_results = await interaction.checks_for(release, caller_data=self.__data)
- if not any(cr.status == sql.CheckResultStatus.CONCERN for cr in check_results):
+ check_results = await interaction.checks_for(
+ release, statuses=(sql.CheckResultStatus.CONCERN,), caller_data=self.__data
+ )
+ if not check_results:
return []
read = storage.Read(self.__write.authorisation, self.__data)
ragp = read.as_general_public()
diff --git a/atr/templates/check-selected-path-table.html b/atr/templates/check-selected-path-table.html
index 70562ff0..57386df7 100644
--- a/atr/templates/check-selected-path-table.html
+++ b/atr/templates/check-selected-path-table.html
@@ -13,7 +13,7 @@
</thead>
<tbody>
{% for path in paths %}
- {% set notes_count = info.notes[path]|length if info else 0 %}
+ {% set notes_count = info.note_counts.get(path, 0) if info else 0 %}
{% set suggestions_count = info.suggestions[path]|length if info else 0 %}
{% set concerns_count = info.concerns[path]|length if info else 0 %}
{% set blockers_count = info.blockers[path]|length if info else 0 %}
diff --git a/tests/unit/test_checker_stats.py b/tests/unit/test_checker_stats.py
index 4b8d370b..e968a4f5 100644
--- a/tests/unit/test_checker_stats.py
+++ b/tests/unit/test_checker_stats.py
@@ -20,6 +20,7 @@ import collections
import datetime
import unittest.mock as mock
+import atr.db.interaction as interaction
import atr.models.safe as safe
import atr.models.sql as sql
import atr.storage.datatypes as datatypes
@@ -28,20 +29,22 @@ import atr.storage.readers.releases as releases
def test_checker_stats_counts_and_files_by_status():
path = safe.RelPath("apache-test-1.0-source.tar.gz")
- success = _make_check_result(sql.CheckResultStatus.NOTE, "Success", primary_rel_path=str(path))
warning = _make_check_result(sql.CheckResultStatus.SUGGESTION, "Warning", primary_rel_path=str(path))
failure = _make_check_result(sql.CheckResultStatus.CONCERN, "Failure", primary_rel_path=str(path))
blocker = _make_check_result(sql.CheckResultStatus.BLOCKER, "Blocker", primary_rel_path=str(path))
info = datatypes.PathInfo(
- notes={path: [success]},
+ note_counts={path: 1},
suggestions={path: [warning]},
concerns={path: [failure]},
blockers={path: [blocker]},
)
+ notes = [
+ interaction.CheckCount(str(path), "atr.tasks.checks.paths.check_errors", sql.CheckResultStatus.NOTE, False, 1)
+ ]
reader = _make_reader()
compute_checker_stats = getattr(reader, "_GeneralPublic__compute_checker_stats")
- compute_checker_stats(info, [path])
+ compute_checker_stats(info, [path], notes)
assert len(info.checker_stats) == 1
stat = info.checker_stats[0]
diff --git a/tests/unit/test_checks_tally.py b/tests/unit/test_checks_tally.py
new file mode 100644
index 00000000..16cfe21d
--- /dev/null
+++ b/tests/unit/test_checks_tally.py
@@ -0,0 +1,103 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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 datetime
+import unittest.mock as mock
+
+import pytest
+import sqlalchemy.ext.asyncio
+import sqlalchemy.pool
+import sqlmodel
+
+import atr.db as db
+import atr.db.interaction as interaction
+import atr.models.safe as safe
+import atr.models.sql as sql
+
+_CHECKER = "atr.tasks.checks.license.headers"
+_PATH = "apache-test-1.0-source.tar.gz"
+
+
+async def test_checks_tally_counts_every_status_and_loads_only_the_requested(monkeypatch: pytest.MonkeyPatch) -> None:
+ engine = sqlalchemy.ext.asyncio.create_async_engine(
+ "sqlite+aiosqlite:///:memory:",
+ connect_args={"check_same_thread": False},
+ poolclass=sqlalchemy.pool.StaticPool,
+ )
+ async with engine.begin() as conn:
+ await conn.run_sync(sqlmodel.SQLModel.metadata.create_all)
+ sessions = sqlalchemy.ext.asyncio.async_sessionmaker(engine, class_=db.Session, expire_on_commit=False)
+ load_checks = mock.AsyncMock(return_value={_PATH: {_CHECKER: "h1"}, "": {"atr.tasks.checks.paths.check": "h2"}})
+ monkeypatch.setattr(interaction.attestable, "load_checks", load_checks)
+ release = sql.Release(phase=sql.ReleasePhase.RELEASE_CANDIDATE, version="1.0", project_key="test")
+ async with sessions() as data:
+ data.add_all(
+ [
+ _result(sql.CheckResultStatus.NOTE, "h1", member_rel_path="a/one.py"),
+ _result(sql.CheckResultStatus.NOTE, "h1", member_rel_path="a/two.py"),
+ _result(sql.CheckResultStatus.CONCERN, "h1", member_rel_path="a/three.py"),
+ _result(sql.CheckResultStatus.NOTE, "h1"),
+ _result(
+ sql.CheckResultStatus.CONCERN, "h2", primary_rel_path=None, checker="atr.tasks.checks.paths.check"
+ ),
+ _result(sql.CheckResultStatus.NOTE, "stale"),
+ ]
+ )
+ await data.commit()
+ tally = await interaction.checks_tally_for(
+ release, safe.RevisionNumber("00001"), statuses=sql.CHECK_RESULT_IGNORABLE_STATUSES, caller_data=data
+ )
+ await engine.dispose()
+
+ expected = [
+ interaction.CheckCount(_PATH, _CHECKER, sql.CheckResultStatus.NOTE, True, 2),
+ interaction.CheckCount(_PATH, _CHECKER, sql.CheckResultStatus.CONCERN, True, 1),
+ interaction.CheckCount(_PATH, _CHECKER, sql.CheckResultStatus.NOTE, False, 1),
+ interaction.CheckCount(None, "atr.tasks.checks.paths.check", sql.CheckResultStatus.CONCERN, False, 1),
+ ]
+ assert sorted(tally.counts, key=_key) == sorted(expected, key=_key)
+ assert [(r.status, r.member_rel_path) for r in tally.results] == [
+ (sql.CheckResultStatus.CONCERN, "a/three.py"),
+ (sql.CheckResultStatus.CONCERN, None),
+ ]
+
+
+def _key(count: interaction.CheckCount) -> tuple[str, str, str, bool]:
+ return (count.primary_rel_path or "", count.checker, count.status.value, count.member)
+
+
+def _result(
+ status: sql.CheckResultStatus,
+ inputs_hash: str,
+ *,
+ primary_rel_path: str | None = _PATH,
+ member_rel_path: str | None = None,
+ checker: str = _CHECKER,
+) -> sql.CheckResult:
+ return sql.CheckResult(
+ release_key="test-1.0",
+ revision_number="00001",
+ checker=checker,
+ checker_version="1",
+ primary_rel_path=primary_rel_path,
+ member_rel_path=member_rel_path,
+ created=datetime.datetime(2026, 1, 1, tzinfo=datetime.UTC),
+ status=status,
+ message="m",
+ data={},
+ inputs_hash=inputs_hash,
+ )
diff --git a/tests/unit/test_get_checks_filestats.py b/tests/unit/test_get_checks_filestats.py
index cf1c040a..de85807a 100644
--- a/tests/unit/test_get_checks_filestats.py
+++ b/tests/unit/test_get_checks_filestats.py
@@ -23,6 +23,7 @@ import unittest.mock as mock
import pytest
+import atr.db.interaction as interaction
import atr.get.checks as checks
import atr.models.safe as safe
import atr.models.sql as sql
@@ -30,18 +31,24 @@ import atr.models.sql as sql
async def test_compute_stats_counts_by_status_before_and_after(monkeypatch: pytest.MonkeyPatch) -> None:
path = safe.RelPath("apache-test-1.0-source.tar.gz")
- check_results = [
- _make_check_result(sql.CheckResultStatus.NOTE, "Success", str(path)),
+ counts = [
+ _make_count(str(path), sql.CheckResultStatus.NOTE, False),
+ _make_count(str(path), sql.CheckResultStatus.SUGGESTION, False),
+ _make_count(str(path), sql.CheckResultStatus.CONCERN, False),
+ _make_count(str(path), sql.CheckResultStatus.BLOCKER, True),
+ _make_count(str(path), sql.CheckResultStatus.EXCEPTION, True),
+ ]
+ ignorable = [
_make_check_result(sql.CheckResultStatus.SUGGESTION, "Warning", str(path)),
_make_check_result(sql.CheckResultStatus.CONCERN, "Failure", str(path)),
- _make_check_result(sql.CheckResultStatus.BLOCKER, "Blocker", str(path), "apache-test-1.0/pom.xml"),
_make_check_result(sql.CheckResultStatus.EXCEPTION, "Exception", str(path), "apache-test-1.0/build.xml"),
]
+ tally = interaction.ChecksTally(counts, ignorable)
release = mock.MagicMock()
release.latest_revision_number = "00001"
monkeypatch.setattr(checks.db, "session", _mock_db_session)
- monkeypatch.setattr(checks.interaction, "checks_for", mock.AsyncMock(return_value=check_results))
+ monkeypatch.setattr(checks.interaction, "checks_tally_for", mock.AsyncMock(return_value=tally))
per_file, totals = await checks._compute_stats(release, [path], _match_ignore)
stats = per_file[path]
@@ -96,6 +103,10 @@ def _make_check_result(
)
+def _make_count(primary_rel_path: str, status: sql.CheckResultStatus, member: bool) -> interaction.CheckCount:
+ return interaction.CheckCount(primary_rel_path, "atr.tasks.checks.paths.check_errors", status, member, 1)
+
+
def _match_ignore(check_result: sql.CheckResult) -> bool:
return check_result.status in (
sql.CheckResultStatus.NOTE,
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org