This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-release.git
The following commit(s) were added to refs/heads/main by this push:
new cc499c5 Convert hashing checks to check result style
cc499c5 is described below
commit cc499c5c8dc6d2b7ac1948e183cb7d80e28d20f7
Author: Sean B. Palmer <[email protected]>
AuthorDate: Mon Mar 31 15:24:45 2025 +0100
Convert hashing checks to check result style
---
atr/routes/draft.py | 23 +++++----
atr/tasks/__init__.py | 11 +++--
atr/tasks/checks/hashing.py | 83 ++++++++++++++++++++++++++++++++
atr/tasks/hashing.py | 75 -----------------------------
atr/templates/draft-review-path.html | 93 ++++++++++++++++++------------------
atr/templates/draft-review.html | 6 +--
atr/worker.py | 14 +++---
7 files changed, 159 insertions(+), 146 deletions(-)
diff --git a/atr/routes/draft.py b/atr/routes/draft.py
index c2559b0..3394256 100644
--- a/atr/routes/draft.py
+++ b/atr/routes/draft.py
@@ -580,15 +580,21 @@ async def review_path(session: routes.CommitterSession,
project_name: str, versi
modified = int(await aiofiles.os.path.getmtime(full_path))
file_size = await aiofiles.os.path.getsize(full_path)
- # Get the most recent task for each task type
- recent_tasks = await db.recent_tasks(data,
f"{project_name}-{version_name}", file_path, modified)
+ # Get all check results for this file
+ query = data.check_result(release_name=release.name,
path=file_path).order_by(
+
db.validate_instrumented_attribute(models.CheckResult.checker).asc(),
+
db.validate_instrumented_attribute(models.CheckResult.created).desc(),
+ )
+ all_results = await query.all()
- # Convert to a list for the template
- tasks = list(recent_tasks.values())
+ # Filter to get only the most recent result for each checker
+ latest_check_results: dict[str, models.CheckResult] = {}
+ for result in all_results:
+ if result.checker not in latest_check_results:
+ latest_check_results[result.checker] = result
- all_tasks_completed = all(
- task.status in (models.TaskStatus.COMPLETED,
models.TaskStatus.FAILED) for task in tasks
- )
+ # Convert to a list for the template
+ check_results_list = list(latest_check_results.values())
file_data = {
"filename": pathlib.Path(file_path).name,
@@ -603,8 +609,7 @@ async def review_path(session: routes.CommitterSession,
project_name: str, versi
file_path=file_path,
package=file_data,
release=release,
- tasks=tasks,
- all_tasks_completed=all_tasks_completed,
+ check_results=check_results_list,
format_file_size=routes.format_file_size,
)
diff --git a/atr/tasks/__init__.py b/atr/tasks/__init__.py
index f1ea785..6abbb6c 100644
--- a/atr/tasks/__init__.py
+++ b/atr/tasks/__init__.py
@@ -20,7 +20,7 @@ import aiofiles.os
import atr.db.models as models
import atr.tasks.checks as checks
import atr.tasks.checks.archive as archive
-import atr.tasks.hashing as hashing
+import atr.tasks.checks.hashing as hashing
import atr.util as util
@@ -74,9 +74,12 @@ async def sha_checks(release: models.Release, hash_file:
str) -> list[models.Tas
tasks.append(
models.Task(
status=models.TaskStatus.QUEUED,
- task_type="verify_file_hash",
+ task_type=checks.function_key(hashing.check),
task_args=hashing.Check(
- original_file=original_file, hash_file=full_hash_file_path,
algorithm=algorithm
+ release_name=release.name,
+ abs_path=original_file,
+ abs_hash_file=full_hash_file_path,
+ algorithm=algorithm,
).model_dump(),
release_name=release.name,
path=hash_file,
@@ -87,7 +90,7 @@ async def sha_checks(release: models.Release, hash_file: str)
-> list[models.Tas
return tasks
-async def tar_gz_checks(release: models.Release, path: str, signature_path:
str | None = None) -> list[models.Task]:
+async def tar_gz_checks(release: models.Release, path: str) ->
list[models.Task]:
# TODO: We should probably use an enum for task_type
full_path = str(util.get_release_candidate_draft_dir() /
release.project.name / release.version / path)
# filename = os.path.basename(path)
diff --git a/atr/tasks/checks/hashing.py b/atr/tasks/checks/hashing.py
new file mode 100644
index 0000000..9508427
--- /dev/null
+++ b/atr/tasks/checks/hashing.py
@@ -0,0 +1,83 @@
+# 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 hashlib
+import logging
+import secrets
+from typing import Final
+
+import aiofiles
+import pydantic
+
+import atr.tasks.checks as checks
+
+_LOGGER: Final = logging.getLogger(__name__)
+
+
+class Check(pydantic.BaseModel):
+ """Parameters for file hash checking."""
+
+ release_name: str = pydantic.Field(..., description="Release name")
+ abs_path: str = pydantic.Field(..., description="Absolute path to the file
to check")
+ abs_hash_file: str = pydantic.Field(..., description="Absolute path to the
hash file")
+ algorithm: str = pydantic.Field(..., description="Hash algorithm to use")
+
+
[email protected]_model(Check)
+async def check(args: Check) -> str | None:
+ """Check the hash of a file."""
+ # This is probably the best idea for the rel_path
+ # But we might want to use the artifact instead
+ rel_path = checks.rel_path(args.abs_hash_file)
+ check_instance = await checks.Check.create(checker=check,
release_name=args.release_name, path=rel_path)
+
+ _LOGGER.info(f"Checking hash ({args.algorithm}) for {args.abs_path}
against {args.abs_hash_file} (rel: {rel_path})")
+
+ if args.algorithm == "sha256":
+ hash_func = hashlib.sha256
+ elif args.algorithm == "sha512":
+ hash_func = hashlib.sha512
+ else:
+ await check_instance.failure(f"Unsupported hash algorithm:
{args.algorithm}", {"algorithm": args.algorithm})
+ return None
+
+ hash_obj = hash_func()
+ try:
+ async with aiofiles.open(args.abs_path, mode="rb") as f:
+ while chunk := await f.read(4096):
+ hash_obj.update(chunk)
+ computed_hash = hash_obj.hexdigest()
+
+ async with aiofiles.open(args.abs_hash_file) as f:
+ expected_hash = await f.read()
+ # May be in the format "HASH FILENAME\n"
+ # TODO: Check the FILENAME part
+ expected_hash = expected_hash.strip().split()[0]
+
+ if secrets.compare_digest(computed_hash, expected_hash):
+ await check_instance.success(
+ f"Hash ({args.algorithm}) matches expected value",
+ {"computed_hash": computed_hash, "expected_hash":
expected_hash},
+ )
+ else:
+ await check_instance.failure(
+ f"Hash ({args.algorithm}) mismatch",
+ {"computed_hash": computed_hash, "expected_hash":
expected_hash},
+ )
+ except Exception as e:
+ await check_instance.failure("Unable to verify hash", {"error":
str(e)})
+ return None
diff --git a/atr/tasks/hashing.py b/atr/tasks/hashing.py
deleted file mode 100644
index 0fbf1a0..0000000
--- a/atr/tasks/hashing.py
+++ /dev/null
@@ -1,75 +0,0 @@
-# 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 hashlib
-import logging
-import secrets
-from typing import Any, Final
-
-import aiofiles
-import pydantic
-
-import atr.db.models as models
-import atr.tasks.task as task
-
-_LOGGER: Final = logging.getLogger(__name__)
-
-
-class Check(pydantic.BaseModel):
- """Parameters for file hash checking."""
-
- original_file: str = pydantic.Field(..., description="Path to the original
file")
- hash_file: str = pydantic.Field(..., description="Path to the hash file")
- algorithm: str = pydantic.Field(..., description="Hash algorithm to use")
-
-
-async def check(args: dict[str, Any]) -> tuple[models.TaskStatus, str | None,
tuple[Any, ...]]:
- """Check the hash of a file."""
- data = Check(**args)
- task_results = task.results_as_tuple(await _check_core(data.original_file,
data.hash_file, data.algorithm))
- _LOGGER.info(f"Verified {data.original_file} and computed size
{task_results[0]}")
- return task.COMPLETED, None, task_results
-
-
-async def _check_core(
- original_file: str, hash_file: str, algorithm: str
-) -> tuple[models.TaskStatus, str | None, tuple[Any, ...]]:
- """Check the hash of a file."""
- if algorithm == "sha256":
- hash_func = hashlib.sha256
- elif algorithm == "sha512":
- hash_func = hashlib.sha512
- else:
- raise task.Error(f"Unsupported hash algorithm: {algorithm}")
- hash_obj = hash_func()
- async with aiofiles.open(original_file, mode="rb") as f:
- while chunk := await f.read(4096):
- hash_obj.update(chunk)
- computed_hash = hash_obj.hexdigest()
- async with aiofiles.open(hash_file) as f:
- expected_hash = await f.read()
- # May be in the format "HASH FILENAME\n"
- # TODO: Check the FILENAME part
- expected_hash = expected_hash.strip().split()[0]
- if secrets.compare_digest(computed_hash, expected_hash):
- return task.COMPLETED, None, ({"computed_hash": computed_hash,
"expected_hash": expected_hash},)
- else:
- return (
- task.FAILED,
- f"Hash mismatch for {original_file}",
- ({"computed_hash": computed_hash, "expected_hash":
expected_hash},),
- )
diff --git a/atr/templates/draft-review-path.html
b/atr/templates/draft-review-path.html
index 5b8feec..43e7883 100644
--- a/atr/templates/draft-review-path.html
+++ b/atr/templates/draft-review-path.html
@@ -31,31 +31,31 @@
<h2>Verification tasks</h2>
- {% if tasks %}
+ {% if check_results %}
<div class="d-flex align-items-center p-3 mb-3 bg-light border rounded">
<span class="fw-bold me-3">Status summary:</span>
<div class="d-flex flex-wrap gap-3">
{% with %}
{% set status_counts = {
- "completed": tasks|selectattr("status.value", "equalto",
"completed")|list|length,
- "failed": tasks|selectattr("status.value", "equalto",
"failed")|list|length,
- "active": tasks|selectattr("status.value", "equalto",
"active")|list|length,
- "queued": tasks|selectattr("status.value", "equalto",
"queued")|list|length
+ "success": check_results|selectattr("status.value",
"equalto", "success")|list|length,
+ "failure": check_results|selectattr("status.value",
"equalto", "failure")|list|length,
+ "warning": check_results|selectattr("status.value",
"equalto", "warning")|list|length,
+ "exception": check_results|selectattr("status.value",
"equalto", "exception")|list|length
} %}
{% for status, count in status_counts.items() %}
{% if count > 0 %}
- <div class="d-flex align-items-center gap-2 px-3 py-2 rounded
fw-medium {% if status == "completed" %}bg-success-subtle border
border-success-subtle {% elif status == "failed" %}bg-danger-subtle border
border-danger-subtle {% elif status == "active" %}bg-info-subtle border
border-info-subtle {% else %}bg-light border{% endif %}">
+ <div class="d-flex align-items-center gap-2 px-3 py-2 rounded
fw-medium {% if status == "success" %}bg-success-subtle border
border-success-subtle {% elif status == "failure" %}bg-danger-subtle border
border-danger-subtle {% elif status == "warning" %}bg-warning-subtle border
border-warning-subtle {% elif status == "exception" %}bg-danger-subtle border
border-danger-subtle {% endif %}">
<span class="fs-5">{{ count }}</span>
<span>
- {%- if status == "queued" -%}
- Pending
- {%- elif status == "active" -%}
- Running
- {%- elif status == "completed" -%}
+ {%- if status == "success" -%}
Passed
- {%- elif status == "failed" -%}
+ {%- elif status == "failure" -%}
Issues
+ {%- elif status == "warning" -%}
+ Warning
+ {%- elif status == "exception" -%}
+ Error
{%- else -%}
{{ status|title }}
{%- endif -%}
@@ -70,55 +70,59 @@
<div class="d-flex gap-3 mb-3">
<button type="button" onclick="toggleAllDetails()" class="btn
btn-secondary">Toggle all details</button>
- {% if tasks and all_tasks_completed %}
- <!-- Currently there's no direct way to restart file checks through the
UI -->
- {% endif %}
+ <!-- TODO: Currently there's no direct way to restart file checks through
the UI -->
</div>
<div class="mb-3">
- {% if tasks %}
- {% for task in tasks %}
+ {% if check_results %}
+ {% for check_result in check_results %}
<div class="border border-2 rounded p-3 mb-3">
<div class="d-flex justify-content-between align-items-center mb-2">
- <span class="fw-bold">{{ task.task_type.replace('_', '
').replace("verify ", "").title() }}</span>
- <span class="badge rounded-pill {% if task.status.value ==
"queued" %}bg-secondary {% elif task.status.value == "active" %}bg-info {% elif
task.status.value == "completed" %}bg-success {% elif task.status.value ==
"failed" %}bg-danger {% else %}bg-secondary{% endif %}">
- {%- if task.status.value == "queued" -%}
- Pending
- {%- elif task.status.value == "active" -%}
- Running
- {%- elif task.status.value == "completed" -%}
+ <span class="fw-bold">{{
function_name_from_key(check_result.checker) }}</span>
+ <span class="badge rounded-pill {% if check_result.status.value ==
"success" %}bg-success {% elif check_result.status.value == "failure"
%}bg-danger {% elif check_result.status.value == "warning" %}bg-warning {% elif
check_result.status.value == "exception" %}bg-danger {% else %}bg-secondary{%
endif %}">
+ {%- if check_result.status.value == "success" -%}
Passed
- {%- elif task.status.value == "failed" -%}
+ {%- elif check_result.status.value == "failure" -%}
Issue
+ {%- elif check_result.status.value == "warning" -%}
+ Warning
+ {%- elif check_result.status.value == "exception" -%}
+ Error
{%- else -%}
- {{ task.status.value }}
+ {{ check_result.status.value|title }}
{%- endif -%}
</span>
</div>
<div class="small">
- <div>Started: {{ task.started.strftime("%Y-%m-%d %H:%M UTC") if
task.started else "Not started" }}</div>
- <div>Completed: {{ task.completed.strftime("%Y-%m-%d %H:%M UTC")
if task.completed else "Not completed" }}</div>
- {% if task.result and task.result[0] is mapping and
task.result|length == 1 %}
+ <div>
+ Checked: {{ check_result.created.strftime("%Y-%m-%d %H:%M UTC")
if check_result.created else "Not checked" }}
+ </div>
+
+ {% if check_result.message %}<div class="mt-2">{{
check_result.message }}</div>{% endif %}
+
+ {% if check_result.data is mapping %}
<details class="mt-2 p-2 bg-light rounded">
- {% if task.error %}
- <summary class="atr-cursor-pointer user-select-none
p-2">Issue: {{ task.error }}</summary>
+ {% if check_result.status.value in ["failure", "exception"] %}
+ <summary class="atr-cursor-pointer user-select-none
p-2">View details ({{ check_result.status.value|title }})</summary>
+ {% elif check_result.status.value == "warning" %}
+ <summary class="atr-cursor-pointer user-select-none
p-2">View details (Warning)</summary>
{% else %}
<summary class="atr-cursor-pointer user-select-none
p-2">View detailed results</summary>
{% endif %}
- {% if task.task_type == "verify_rat_license" and
task.result[0] is mapping %}
+ {% if check_result.checker.endswith("rat.check_licenses") %}
<div class="d-flex gap-3 mb-2">
<span class="badge bg-success-subtle text-success-emphasis
border border-success-subtle px-2 py-1">
- <strong>{{ task.result[0].approved_licenses }}</strong>
files with approved licenses
+ <strong>{{ check_result.data.get('approved_licenses', 0)
}}</strong> files with approved licenses
</span>
- {% if task.result[0].unapproved_licenses > 0 %}
+ {% if check_result.data.get("unapproved_licenses", 0) > 0
%}
<span class="badge bg-danger-subtle text-danger-emphasis
border border-danger-subtle px-2 py-1">
- <strong>{{ task.result[0].unapproved_licenses
}}</strong> files with unapproved licenses
+ <strong>{{
check_result.data.get('unapproved_licenses', 0) }}</strong> files with
unapproved licenses
</span>
{% endif %}
- {% if task.result[0].unknown_licenses > 0 %}
+ {% if check_result.data.get("unknown_licenses", 0) > 0 %}
<span class="badge bg-warning-subtle
text-warning-emphasis border border-warning-subtle px-2 py-1">
- <strong>{{ task.result[0].unknown_licenses }}</strong>
files with unknown licenses
+ <strong>{{ check_result.data.get('unknown_licenses',
0) }}</strong> files with unknown licenses
</span>
{% endif %}
</div>
@@ -126,10 +130,10 @@
<table class="table table-bordered mt-2">
<tbody>
- {% for key, value in task.result[0].items() %}
+ {% for key, value in check_result.data.items() %}
{% if key != "debug_info" %}
<tr>
- <th class="bg-light fw-bold">{{ key|replace('_', '
') |title }}</th>
+ <th class="bg-light fw-bold align-top">{{
key|replace('_', ' ') |title }}</th>
<td>
{% if value is boolean %}
{{ "Yes" if value else "No" }}
@@ -175,13 +179,6 @@
</tbody>
</table>
</details>
- {% else %}
- {% if task.error %}<div class="mt-2 p-2 bg-light rounded">Issue:
{{ task.error }}</div>{% endif %}
- {% if task.result %}
- <div class="mt-2 p-2 bg-light rounded">
- <pre class="mb-0">{{ task.result }}</pre>
- </div>
- {% endif %}
{% endif %}
</div>
</div>
@@ -205,3 +202,7 @@
}
</script>
{% endblock javascripts %}
+
+{% macro function_name_from_key(key) -%}
+ {{- key.split(".")[-1] .replace("_", " ") | title -}}
+{%- endmacro %}
diff --git a/atr/templates/draft-review.html b/atr/templates/draft-review.html
index 23d4024..36b9bd8 100644
--- a/atr/templates/draft-review.html
+++ b/atr/templates/draft-review.html
@@ -94,10 +94,8 @@
class="btn btn-sm btn-outline-primary fs-6">Download</a>
<a href="{{ as_url(routes.draft.tools,
project_name=project_name, version_name=version_name, file_path=path) }}"
class="btn btn-sm btn-outline-primary fs-6 small
ms-2">Tools</a>
- {% if path in tasks and tasks[path]|length > 0 %}
- <a href="{{ as_url(routes.draft.review_path,
project_name=project_name, version_name=version_name, file_path=path) }}"
- class="btn btn-sm btn-outline-primary fs-6 small
ms-2">Review file</a>
- {% endif %}
+ <a href="{{ as_url(routes.draft.review_path,
project_name=project_name, version_name=version_name, file_path=path) }}"
+ class="btn btn-sm btn-outline-primary fs-6 small
ms-2">Review file</a>
</td>
</tr>
{% endfor %}
diff --git a/atr/worker.py b/atr/worker.py
index c621960..5fdb84a 100644
--- a/atr/worker.py
+++ b/atr/worker.py
@@ -38,7 +38,9 @@ import sqlmodel
import atr.db as db
import atr.db.models as models
import atr.tasks.bulk as bulk
-import atr.tasks.hashing as hashing
+import atr.tasks.checks as checks
+import atr.tasks.checks.archive as archive
+import atr.tasks.checks.hashing as hashing
import atr.tasks.license as license
import atr.tasks.mailtest as mailtest
import atr.tasks.rat as rat
@@ -181,22 +183,18 @@ async def _task_result_process(
async def _task_process(task_id: int, task_type: str, task_args: list[str] |
dict[str, Any]) -> None:
"""Process a claimed task."""
- import atr.tasks.checks as checks
- import atr.tasks.checks.archive as checks_archive
-
_LOGGER.info(f"Processing task {task_id} ({task_type}) with args
{task_args}")
try:
# Map task types to their handler functions
modern_task_handlers: dict[str, Callable[..., Awaitable[str | None]]]
= {
- checks.function_key(checks_archive.integrity):
checks_archive.integrity,
- checks.function_key(checks_archive.structure):
checks_archive.structure,
+ checks.function_key(archive.integrity): archive.integrity,
+ checks.function_key(archive.structure): archive.structure,
+ checks.function_key(hashing.check): hashing.check,
}
# TODO: We should use a decorator to register these automatically
dict_task_handlers = {
- # "verify_archive_integrity": archive.check_integrity,
"package_bulk_download": bulk.download,
"rsync_analyse": rsync.analyse,
- "verify_file_hash": hashing.check,
}
# TODO: These are synchronous
# We plan to convert these to async dict handlers
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
