This is an automated email from the ASF dual-hosted git repository. arm pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
commit 9b963adedca66cb88e5354fbafbff438e639ccdf Author: Alastair McFarlane <[email protected]> AuthorDate: Mon Feb 23 10:26:38 2026 +0000 Refactor check get logic to a shared method and remove some extra places where we still used release_name or version. Remove bulk-delete of check results. Update documentation. --- atr/api/__init__.py | 23 ++--------- atr/attestable.py | 5 ++- atr/db/interaction.py | 85 +++++++++++++++++++++++++++++------------ atr/docs/checks.md | 4 +- atr/get/checks.py | 14 +------ atr/storage/readers/checks.py | 27 +------------ atr/storage/readers/releases.py | 14 ++----- atr/storage/writers/release.py | 9 ----- atr/tasks/checks/__init__.py | 10 ++--- 9 files changed, 82 insertions(+), 109 deletions(-) diff --git a/atr/api/__init__.py b/atr/api/__init__.py index 4a0321ce..a582cffc 100644 --- a/atr/api/__init__.py +++ b/atr/api/__init__.py @@ -29,7 +29,6 @@ import sqlalchemy import sqlmodel import werkzeug.exceptions as exceptions -import atr.attestable as attestable import atr.blueprints.api as api import atr.config as config import atr.db as db @@ -73,27 +72,17 @@ async def checks_list(project: str, version: str) -> DictResponse: may potentially be thousands or results or more. """ # TODO: We should perhaps paginate this - # TODO: Add phase in the response, and the revision too _simple_check(project, version) - # TODO: Merge with checks_list_project_version_revision async with db.session() as data: release_name = sql.release_name(project, version) release = await data.release(name=release_name).demand(exceptions.NotFound(f"Release {release_name} not found")) - if not release.latest_revision_number: - raise exceptions.InternalServerError("No latest revision found") - file_path_checks = await attestable.load_checks(project, version, release.latest_revision_number) - if file_path_checks: - check_results = await data.check_result( - inputs_hash_in=[h for inner in file_path_checks.values() for h in inner.values()] - ).all() - else: - check_results = [] + check_results = await interaction.checks_for(release, caller_data=data) return models.api.ChecksListResults( endpoint="/checks/list", checks=check_results, - checks_revision=release.latest_revision_number, + checks_revision=release.unwrap_revision_number, current_phase=release.phase, ).model_dump(), 200 @@ -127,13 +116,7 @@ async def checks_list_revision(project: str, version: str, revision: str) -> Dic if revision_result is None: raise exceptions.NotFound(f"Revision '{revision}' does not exist for release '{project}-{version}'") - file_path_checks = await attestable.load_checks(project, version, revision) - if file_path_checks: - check_results = await data.check_result( - inputs_hash_in=[h for inner in file_path_checks.values() for h in inner.values()] - ).all() - else: - check_results = [] + check_results = await interaction.checks_for(release_result, revision=revision, caller_data=data) return models.api.ChecksListResults( endpoint="/checks/list", diff --git a/atr/attestable.py b/atr/attestable.py index 5bedab50..9f80690d 100644 --- a/atr/attestable.py +++ b/atr/attestable.py @@ -97,8 +97,10 @@ async def load_checks( project_name: str, version_name: str, revision_number: str, -) -> dict[str, dict[str, str]] | None: +) -> dict[str, dict[str, str]]: file_path = attestable_checks_path(project_name, version_name, revision_number) + # TODO: Once we're sure everyone is on V2, we should be strict about failures here, + # rather than returning {} if await aiofiles.os.path.isfile(file_path): try: async with aiofiles.open(file_path, encoding="utf-8") as f: @@ -109,6 +111,7 @@ async def load_checks( return models.AttestableChecksV2.model_validate(data).checks except (json.JSONDecodeError, pydantic.ValidationError) as e: log.warning(f"Could not parse {file_path}: {e}") + return {} return {} diff --git a/atr/db/interaction.py b/atr/db/interaction.py index 9854f2e9..bb3ff3ac 100644 --- a/atr/db/interaction.py +++ b/atr/db/interaction.py @@ -26,6 +26,7 @@ import sqlalchemy import sqlalchemy.orm as orm import sqlmodel +import atr.attestable as attestable import atr.db as db import atr.jwtoken as jwtoken import atr.ldap as ldap @@ -160,6 +161,37 @@ async def candidates(project: sql.Project) -> list[sql.Release]: return await releases_by_phase(project, sql.ReleasePhase.RELEASE_CANDIDATE) +async def checks_for( + release: sql.Release, + revision: str | None = None, + rel_path: str | None = None, + caller_data: db.Session | None = None, +) -> list[sql.CheckResult]: + """Get the check results for a release, optionally for a specific revision and/or file path.""" + if revision is None: + revision = release.unwrap_revision_number + file_path_checks = await attestable.load_checks(release.project_name, release.version, revision) + if file_path_checks: + if rel_path is not None: + hashes = [ + h for key in ("", str(rel_path)) if key in file_path_checks for h in file_path_checks[key].values() + ] + else: + hashes = [h for inner in file_path_checks.values() for h in inner.values()] + async with db.ensure_session(caller_data) as data: + check_results = ( + await data.check_result(inputs_hash_in=hashes, primary_rel_path=rel_path or db.NOT_SET) + .order_by( + sql.validate_instrumented_attribute(sql.CheckResult.checker).asc(), + sql.validate_instrumented_attribute(sql.CheckResult.created).desc(), + ) + .all() + ) + else: + check_results = [] + return list(check_results) + + @contextlib.asynccontextmanager async def ephemeral_gpg_home() -> AsyncGenerator[str]: """Create a temporary directory for an isolated GPG home, and clean it up on exit.""" @@ -173,33 +205,17 @@ async def full_releases(project: sql.Project) -> list[sql.Release]: async def has_blocker_checks(release: sql.Release, revision_number: str, caller_data: db.Session | None = None) -> bool: - async with db.ensure_session(caller_data) as data: - query = ( - sqlmodel.select(sqlalchemy.func.count()) - .select_from(sql.CheckResult) - .where( - sql.CheckResult.release_name == release.name, - sql.CheckResult.revision_number == revision_number, - sql.CheckResult.status == sql.CheckResultStatus.BLOCKER, - ) - ) - result = await data.execute(query) - return result.scalar_one() > 0 + count = await count_checks_for_revision_by_status( + sql.CheckResultStatus.BLOCKER, release, revision_number, caller_data + ) + return count > 0 async def has_failing_checks(release: sql.Release, revision_number: str, caller_data: db.Session | None = None) -> bool: - async with db.ensure_session(caller_data) as data: - query = ( - sqlmodel.select(sqlalchemy.func.count()) - .select_from(sql.CheckResult) - .where( - sql.CheckResult.release_name == release.name, - sql.CheckResult.revision_number == revision_number, - sql.CheckResult.status == sql.CheckResultStatus.FAILURE, - ) - ) - result = await data.execute(query) - return result.scalar_one() > 0 + count = await count_checks_for_revision_by_status( + sql.CheckResultStatus.FAILURE, release, revision_number, caller_data + ) + return count > 0 async def latest_info(project_name: str, version_name: str) -> tuple[str, str, datetime.datetime] | None: @@ -534,6 +550,27 @@ async def wait_for_task( return False +async def count_checks_for_revision_by_status( + status: sql.CheckResultStatus, release: sql.Release, revision_number: str, caller_data: db.Session | None = None +): + file_path_checks = await attestable.load_checks(release.project_name, release.version, revision_number) + check_hashes = [h for inner in file_path_checks.values() for h in inner.values()] + if len(check_hashes) == 0: + return 0 + async with db.ensure_session(caller_data) as data: + via = sql.validate_instrumented_attribute + query = ( + sqlmodel.select(sqlalchemy.func.count()) + .select_from(sql.CheckResult) + .where( + via(sql.CheckResult.inputs_hash).in_(check_hashes), + sql.CheckResult.status == status, + ) + ) + result = await data.execute(query) + return result.scalar_one() + + async def _trusted_project(repository: str, workflow_ref: str, phase: TrustedProjectPhase) -> sql.Project: # Debugging log.info(f"GitHub OIDC JWT payload: {repository} {workflow_ref}") diff --git a/atr/docs/checks.md b/atr/docs/checks.md index 30badd04..e3ec619c 100644 --- a/atr/docs/checks.md +++ b/atr/docs/checks.md @@ -106,9 +106,9 @@ SBOM tasks record task results rather than check results, so there is no checker ## Check caching and reruns -To save time, ATR will sometimes reuse results from a prior revision when a file has not changed. The system computes a content hash and copies earlier results so that the same check does not run again. +To save time, ATR caches check results based on a hash of the check inputs, and will therefore sometimes reuse results from a prior run if the file is identical. -_For debugging only_, if you need a fresh run, place a file named `.atr-no-cache` in the revision root. When that file is present, ATR does not reuse cached results for that revision. We may remove this functionality. +_For debugging only_, an admin can force a cache bust by clicking the "Disable global cache" button in the compose phase, which will add a release-specific suffix to the cache key, forcing a re-run. ## Project policy inputs diff --git a/atr/get/checks.py b/atr/get/checks.py index e6297e9d..36b7b4dc 100644 --- a/atr/get/checks.py +++ b/atr/get/checks.py @@ -23,7 +23,6 @@ import asfquart.base as base import htpy import quart -import atr.attestable as attestable import atr.blueprints.get as get import atr.db as db import atr.db.interaction as interaction @@ -233,17 +232,8 @@ async def _compute_stats( # noqa: C901 empty_stats = FileStats(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) return {p: empty_stats for p in paths}, empty_stats - file_path_checks = await attestable.load_checks( - release.project_name, release.version, release.latest_revision_number - ) - - if file_path_checks: - async with db.session() as data: - check_results = await data.check_result( - inputs_hash_in=[h for inner in file_path_checks.values() for h in inner.values()] - ).all() - else: - check_results = [] + async with db.session() as data: + check_results = await interaction.checks_for(release, caller_data=data) for cr in check_results: if not cr.primary_rel_path: diff --git a/atr/storage/readers/checks.py b/atr/storage/readers/checks.py index 26bc4ac5..c5446ad6 100644 --- a/atr/storage/readers/checks.py +++ b/atr/storage/readers/checks.py @@ -20,8 +20,8 @@ from __future__ import annotations from typing import TYPE_CHECKING -import atr.attestable as attestable import atr.db as db +import atr.db.interaction as interaction import atr.models.sql as sql import atr.storage as storage import atr.storage.types as types @@ -49,30 +49,7 @@ class GeneralPublic: if release.latest_revision_number is None: raise ValueError("Release has no revision - Invalid state") - file_path_checks = await attestable.load_checks( - release.project_name, release.version, release.latest_revision_number - ) - all_check_results = ( - [ - a - for a in await self.__data.check_result( - inputs_hash_in=[ - h - for key in ("", str(rel_path)) - if key in file_path_checks - for h in file_path_checks[key].values() - ], - primary_rel_path=str(rel_path), - ) - .order_by( - sql.validate_instrumented_attribute(sql.CheckResult.checker).asc(), - sql.validate_instrumented_attribute(sql.CheckResult.created).desc(), - ) - .all() - ] - if file_path_checks - else [] - ) + all_check_results = await interaction.checks_for(release, rel_path=str(rel_path), caller_data=self.__data) # Filter out any results that are ignored unignored_checks = [] diff --git a/atr/storage/readers/releases.py b/atr/storage/readers/releases.py index e0da2459..18a48a56 100644 --- a/atr/storage/readers/releases.py +++ b/atr/storage/readers/releases.py @@ -21,9 +21,9 @@ from __future__ import annotations import dataclasses import pathlib -import atr.attestable as attestable import atr.classify as classify import atr.db as db +import atr.db.interaction as interaction import atr.models.sql as sql import atr.storage as storage import atr.storage.types as types @@ -123,16 +123,8 @@ class GeneralPublic: self, release: sql.Release, latest_revision_number: str, info: types.PathInfo ) -> None: match_ignore = await self.__read_as.checks.ignores_matcher(release.project_name) - file_path_checks = await attestable.load_checks(release.project_name, release.version, latest_revision_number) - attestable_checks = ( - [ - a - for a in await self.__data.check_result( - inputs_hash_in=[h for inner in file_path_checks.values() for h in inner.values()] - ).all() - ] - if file_path_checks - else [] + attestable_checks = await interaction.checks_for( + release, revision=latest_revision_number, caller_data=self.__data ) cs = types.ChecksSubset( diff --git a/atr/storage/writers/release.py b/atr/storage/writers/release.py index a937b207..b9970e81 100644 --- a/atr/storage/writers/release.py +++ b/atr/storage/writers/release.py @@ -122,15 +122,6 @@ class CommitteeParticipant(FoundationCommitter): task_count = task_result.rowcount if isinstance(task_result, engine.CursorResult) else 0 log.debug(f"Deleted {util.plural(task_count, 'task')} for {project_name} {version}") - # Bulk delete check results - # Handled by cascade, but we do this explicitly anyway - check_delete_stmt = sqlmodel.delete(sql.CheckResult).where( - via(sql.CheckResult.release_name) == release.name, - ) - check_result = await self.__data.execute(check_delete_stmt) - check_count = check_result.rowcount if isinstance(check_result, engine.CursorResult) else 0 - log.debug(f"Deleted {util.plural(check_count, 'check result')} for {project_name} {version}") - release_name = release.name await self.__data.delete(release) log.info(f"Deleted release record: {project_name} {version}") diff --git a/atr/tasks/checks/__init__.py b/atr/tasks/checks/__init__.py index 45573245..a0335aa6 100644 --- a/atr/tasks/checks/__init__.py +++ b/atr/tasks/checks/__init__.py @@ -215,11 +215,8 @@ class Recorder: async def clear(self, primary_rel_path: str | None = None, member_rel_path: str | None = None) -> None: async with db.session() as data: stmt = sqlmodel.delete(sql.CheckResult).where( - sql.validate_instrumented_attribute(sql.CheckResult.release_name) == self.release_name, - sql.validate_instrumented_attribute(sql.CheckResult.revision_number) == self.revision_number, - sql.validate_instrumented_attribute(sql.CheckResult.checker) == self.checker, + sql.validate_instrumented_attribute(sql.CheckResult.inputs_hash) == self.input_hash, sql.validate_instrumented_attribute(sql.CheckResult.primary_rel_path) == primary_rel_path, - sql.validate_instrumented_attribute(sql.CheckResult.member_rel_path) == member_rel_path, ) await data.execute(stmt) await data.commit() @@ -437,7 +434,10 @@ async def _resolve_unsuffixed_file_hash(release: sql.Release, rel_path: str | No release.project_name, release.version, release.latest_revision_number, rel_path ) plain_path = abs_path.with_suffix("") - return await hashes.compute_file_hash(plain_path) + if await aiofiles.os.path.isfile(plain_path): + return await hashes.compute_file_hash(plain_path) + else: + return "" _EXTRA_ARG_RESOLVERS: Final[dict[str, Callable[[sql.Release, str | None], Any]]] = { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
