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-atr-experiments.git
The following commit(s) were added to refs/heads/main by this push:
new 5d29f5a Add a form field to select the artifact type
5d29f5a is described below
commit 5d29f5acc5902b5f90e059f785870588aee86cb1
Author: Sean B. Palmer <s...@miscoranda.com>
AuthorDate: Thu Feb 20 16:10:40 2025 +0200
Add a form field to select the artifact type
---
atr/db/models.py | 2 +
atr/routes.py | 109 +++++++++++++--------
atr/templates/candidate-attach.html | 49 ++++++++-
docs/plan.html | 7 +-
docs/plan.md | 7 +-
...al_schema.py => 6776f795ec62_initial_schema.py} | 6 +-
6 files changed, 128 insertions(+), 52 deletions(-)
diff --git a/atr/db/models.py b/atr/db/models.py
index 4c07ae6..226b73f 100644
--- a/atr/db/models.py
+++ b/atr/db/models.py
@@ -137,6 +137,8 @@ class Package(SQLModel, table=True):
# The SHA3-256 hash of the file, used as filename in storage
# TODO: We should discuss making this unique
artifact_sha3: str = Field(primary_key=True)
+ # The type of artifact (source, binary, reproducible binary)
+ artifact_type: str
# Original filename from uploader
filename: str
# SHA-512 hash of the file
diff --git a/atr/routes.py b/atr/routes.py
index 2ef95a5..f6d8ba6 100644
--- a/atr/routes.py
+++ b/atr/routes.py
@@ -99,52 +99,23 @@ async def ephemeral_gpg_home():
async def release_attach_post(session: ClientSession, request: Request) ->
Response:
"""Handle POST request for attaching package artifacts to a release."""
-
- async def flash_error_and_redirect(message: str) -> Response:
- await flash(message, "error")
- return redirect(url_for("root_candidate_attach"))
-
- form = await request.form
-
- # TODO: Check that the submitter is a committer of the project
-
- release_key = form.get("release_key")
- if not release_key:
- return await flash_error_and_redirect("Release key is required")
-
- # Get all uploaded files
- files = await request.files
-
- # Get the release artifact, checksum, and signature files
- artifact_file = files.get("release_artifact")
- checksum_file = files.get("release_checksum")
- signature_file = files.get("release_signature")
- if not isinstance(artifact_file, FileStorage):
- return await flash_error_and_redirect("Release artifact file is
required")
+ res = await release_attach_post_validate(request)
+ # Not particularly elegant
+ if isinstance(res, Response):
+ return res
+ release_key, artifact_file, checksum_file, signature_file, artifact_type =
res
+ # This must come here to appease the type checker
if artifact_file.filename is None:
- return await flash_error_and_redirect("Release artifact filename is
required")
- if checksum_file is not None and not isinstance(checksum_file,
FileStorage):
- return await flash_error_and_redirect("Problem with checksum file")
- if signature_file is not None and not isinstance(signature_file,
FileStorage):
- return await flash_error_and_redirect("Problem with signature file")
+ await flash("Release artifact filename is required", "error")
+ return redirect(url_for("root_candidate_attach"))
# Save files and create package record in one transaction
async with get_session() as db_session:
async with db_session.begin():
- # First check for duplicates
- statement = select(Package).where(
- Package.release_key == release_key,
- Package.filename == artifact_file.filename,
- )
- duplicate = (await db_session.execute(statement)).first()
-
- if duplicate:
- return await flash_error_and_redirect("This release artifact
has already been uploaded")
-
# Process and save the files
try:
- ok, artifact_sha3, artifact_size, artifact_sha512,
signature_sha3 = await release_attach_post_helper(
- artifact_file, checksum_file, signature_file
+ ok, artifact_sha3, artifact_size, artifact_sha512,
signature_sha3 = await release_attach_post_session(
+ db_session, release_key, artifact_file, checksum_file,
signature_file
)
if not ok:
# The flash error is already set by the helper
@@ -153,6 +124,7 @@ async def release_attach_post(session: ClientSession,
request: Request) -> Respo
# Create the package record
package = Package(
artifact_sha3=artifact_sha3,
+ artifact_type=artifact_type,
filename=artifact_file.filename,
signature_sha3=signature_sha3,
sha512=artifact_sha512,
@@ -163,16 +135,69 @@ async def release_attach_post(session: ClientSession,
request: Request) -> Respo
db_session.add(package)
except Exception as e:
- return await flash_error_and_redirect(f"Error processing
files: {e!s}")
+ await flash(f"Error processing files: {e!s}", "error")
+ return redirect(url_for("root_candidate_attach"))
# Otherwise redirect to review page
return redirect(url_for("root_candidate_review"))
-async def release_attach_post_helper(
- artifact_file: FileStorage, checksum_file: FileStorage | None,
signature_file: FileStorage | None
+async def release_attach_post_validate(
+ request: Request,
+) -> Response | tuple[str, FileStorage, FileStorage | None, FileStorage |
None, str]:
+ async def flash_error_and_redirect(message: str) -> Response:
+ await flash(message, "error")
+ return redirect(url_for("root_candidate_attach"))
+
+ form = await request.form
+
+ # TODO: Check that the submitter is a committer of the project
+
+ release_key = form.get("release_key")
+ if (not release_key) or (not isinstance(release_key, str)):
+ return await flash_error_and_redirect("Release key is required")
+
+ # Get all uploaded files
+ files = await request.files
+ artifact_file = files.get("release_artifact")
+ checksum_file = files.get("release_checksum")
+ signature_file = files.get("release_signature")
+ if not isinstance(artifact_file, FileStorage):
+ return await flash_error_and_redirect("Release artifact file is
required")
+ if checksum_file is not None and not isinstance(checksum_file,
FileStorage):
+ return await flash_error_and_redirect("Problem with checksum file")
+ if signature_file is not None and not isinstance(signature_file,
FileStorage):
+ return await flash_error_and_redirect("Problem with signature file")
+
+ # Get and validate artifact type
+ artifact_type = form.get("artifact_type")
+ if (not artifact_type) or (not isinstance(artifact_type, str)):
+ return await flash_error_and_redirect("Artifact type is required")
+ if artifact_type not in ["source", "binary", "reproducible"]:
+ return await flash_error_and_redirect("Invalid artifact type")
+
+ return release_key, artifact_file, checksum_file, signature_file,
artifact_type
+
+
+async def release_attach_post_session(
+ db_session: AsyncSession,
+ release_key: str,
+ artifact_file: FileStorage,
+ checksum_file: FileStorage | None,
+ signature_file: FileStorage | None,
) -> tuple[bool, str, int, str, str | None]:
"""Helper function for release_attach_post."""
+ # First check for duplicates
+ statement = select(Package).where(
+ Package.release_key == release_key,
+ Package.filename == artifact_file.filename,
+ )
+ duplicate = (await db_session.execute(statement)).first()
+
+ if duplicate:
+ await flash("This release artifact has already been uploaded", "error")
+ return False, "", 0, "", None
+
# Save files using their hashes as filenames
uploads_path = Path(get_release_storage_dir())
try:
diff --git a/atr/templates/candidate-attach.html
b/atr/templates/candidate-attach.html
index c92c8e2..010d0c6 100644
--- a/atr/templates/candidate-attach.html
+++ b/atr/templates/candidate-attach.html
@@ -82,6 +82,23 @@
border: 1px solid #c3e6cb;
color: #155724;
}
+
+ .radio-group {
+ margin-bottom: 0.5rem;
+ }
+
+ .radio-group div {
+ margin: 0.5rem 0;
+ }
+
+ .radio-group label {
+ margin-left: 0.5rem;
+ cursor: pointer;
+ }
+
+ .radio-group input[type="radio"] {
+ cursor: pointer;
+ }
</style>
{% endblock stylesheets %}
@@ -94,7 +111,12 @@
{% with messages = get_flashed_messages(with_categories=true) %}
{% if messages %}
- {% for category, message in messages %}<div class="flash-message
flash-{{ category }}">{{ message }}</div>{% endfor %}
+ {% for category, message in messages %}
+ <div class="flash-message flash-{{ category }}">
+ {% if category == 'error' %}<strong>Error:</strong>{% endif %}
+ {{ message }}
+ </div>
+ {% endfor %}
{% endif %}
{% endwith %}
@@ -139,6 +161,31 @@
</td>
</tr>
+ <tr>
+ <th>
+ <label>Artifact Type:</label>
+ </th>
+ <td>
+ <div class="radio-group">
+ <div>
+ <input type="radio" id="source" name="artifact_type"
value="source" required />
+ <label for="source">Source archive</label>
+ </div>
+ <div>
+ <input type="radio" id="binary" name="artifact_type"
value="binary" />
+ <label for="binary">Binary archive</label>
+ </div>
+ <div>
+ <input type="radio"
+ id="reproducible"
+ name="artifact_type"
+ value="reproducible" />
+ <label for="reproducible">Reproducible binary archive</label>
+ </div>
+ </div>
+ </td>
+ </tr>
+
<tr>
<th>
<label for="release_checksum">SHA-512 Checksum File:</label>
diff --git a/docs/plan.html b/docs/plan.html
index ffccd1e..782115c 100644
--- a/docs/plan.html
+++ b/docs/plan.html
@@ -5,10 +5,11 @@
<li>
<p>Improve RC workflow</p>
<ul>
-<li>Allow upload of checksum file alongside artifacts and signatures</li>
-<li>Add a checkbox to choose the RC artifact type</li>
-<li>Allow extra types of artifact, such as reproducible binary and convenience
binary</li>
+<li>[DONE] Allow upload of checksum file alongside artifacts and
signatures</li>
+<li>[DONE] Add a form field to choose the RC artifact type</li>
+<li>[DONE] Allow extra types of artifact, such as reproducible binary and
convenience binary</li>
<li>Differentiate between podling PPMCs and top level PMCs</li>
+<li>Allow package deletion</li>
</ul>
</li>
<li>
diff --git a/docs/plan.md b/docs/plan.md
index 40fc238..ad55597 100644
--- a/docs/plan.md
+++ b/docs/plan.md
@@ -5,10 +5,11 @@ This is a rough plan of immediate tasks. The priority of
these tasks may change,
## UX improvements
1. Improve RC workflow
- - Allow upload of checksum file alongside artifacts and signatures
- - Add a checkbox to choose the RC artifact type
- - Allow extra types of artifact, such as reproducible binary and
convenience binary
+ - [DONE] Allow upload of checksum file alongside artifacts and signatures
+ - [DONE] Add a form field to choose the RC artifact type
+ - [DONE] Allow extra types of artifact, such as reproducible binary and
convenience binary
- Differentiate between podling PPMCs and top level PMCs
+ - Allow package deletion
2. Enhance RC display
- [DONE] Augment raw file hashes with the original filenames in the UI
diff --git a/migrations/versions/d8434a3de3da_initial_schema.py
b/migrations/versions/6776f795ec62_initial_schema.py
similarity index 84%
rename from migrations/versions/d8434a3de3da_initial_schema.py
rename to migrations/versions/6776f795ec62_initial_schema.py
index 82f8c3a..3f1e9ef 100644
--- a/migrations/versions/d8434a3de3da_initial_schema.py
+++ b/migrations/versions/6776f795ec62_initial_schema.py
@@ -1,15 +1,15 @@
"""initial_schema
-Revision ID: d8434a3de3da
+Revision ID: 6776f795ec62
Revises:
-Create Date: 2025-02-20 15:24:43.391427
+Create Date: 2025-02-20 16:02:41.618093
"""
from collections.abc import Sequence
# revision identifiers, used by Alembic.
-revision: str = "d8434a3de3da"
+revision: str = "6776f795ec62"
down_revision: str | None = None
branch_labels: str | Sequence[str] | None = None
depends_on: str | Sequence[str] | None = None
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tooling.apache.org
For additional commands, e-mail: dev-h...@tooling.apache.org
