asf-tooling commented on issue #507:
URL:
https://github.com/apache/tooling-trusted-releases/issues/507#issuecomment-4410192252
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@2da7807a`
**Type:** `new_feature` • **Classification:** `actionable` •
**Confidence:** `medium`
**Application domain(s):** `release_lifecycle`, `admin_operations`
### Summary
Issue #507 requests that finished/announced releases can only be archived
(not deleted), with a clear state representation. @dave2wave clarified: 'Once a
release is finished and announced it may only be archived.' Currently, the
`ReleasePhase` enum has no `ARCHIVED` state, and the admin delete functionality
allows deletion of releases in any phase. This requires adding an ARCHIVED
phase, restricting deletion of RELEASE-phase releases, and adding an archive
action.
### Where this lives in the code today
#### `atr/storage/writers/release.py` — `CommitteeParticipant.delete` (lines
168-178)
_needs modification_
The delete method accepts an optional phase filter but does not enforce the
rule that RELEASE-phase releases cannot be deleted.
```python
async def delete(
self,
project_key: safe.ProjectKey,
version: safe.VersionKey,
phase: db.Opt[sql.ReleasePhase] = db.NOT_SET,
include_downloads: bool = True,
) -> str | None:
"""Handle the deletion of database records and filesystem data for a
release."""
release = await self.__data.release(
project_key=str(project_key), version=str(version), phase=phase,
_committee=True
).demand(storage.AccessError(f"Release '{project_key!s} {version!s}'
not found.", status=404))
```
#### `atr/post/draft.py` — `delete` (lines 40-60)
_currently does this_
The draft delete endpoint already correctly restricts deletion to
RELEASE_CANDIDATE_DRAFT phase only.
```python
@post.typed
async def delete(
session: web.Committer,
_draft_delete: Literal["draft/delete"],
project_key: safe.ProjectKey,
version_key: safe.VersionKey,
_form: form.Empty,
) -> web.WerkzeugResponse:
"""
URL: /draft/delete/<project_key>/<version_key>
Delete a candidate draft and all its associated files.
"""
# Delete the metadata from the database
async with storage.write(session) as write:
wacp = await write.as_project_committee_participant(project_key)
error = await wacp.release.delete(
project_key,
version_key,
phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT,
include_downloads=False,
)
```
#### `atr/admin/__init__.py` — `delete_release_get` (lines 382-411)
_needs modification_
The admin delete form shows all releases regardless of phase - should
exclude RELEASE-phase releases or mark them as archive-only.
```python
async def delete_release_get(
_session: web.Committer, _delete_release_get: Literal["delete-release"]
) -> str | web.WerkzeugResponse:
"""
URL: GET /delete-release
Display the form to delete releases.
"""
async with db.session() as data:
releases = await
data.release(_project=True).order_by(sql.Release.key).all()
if releases:
releases_widget = htpy.div[
[
htpy.div(".form-check")[
htpy.input(
class_="form-check-input",
type="checkbox",
name="releases_to_delete",
value=release.key,
id=f"release_{release.key}",
),
htpy.label(".form-check-label",
for_=f"release_{release.key}")[
htpy.strong[release.key],
f" ({release.project.display_name},
{release.phase.value.upper()})",
],
]
for release in releases
]
]
```
#### `atr/admin/__init__.py` — `delete_release_post` (lines 430-440)
_needs modification_
The admin delete POST handler should guard against deleting RELEASE-phase
releases.
```python
async def delete_release_post(
session: web.Committer, _delete_release_get: Literal["delete-release"],
delete_form: DeleteReleaseForm
) -> str | web.WerkzeugResponse:
"""
URL: POST /delete-release
Delete selected releases and their associated data and files.
"""
await _delete_releases(session, delete_form.releases_to_delete)
return await session.redirect(delete_release_get)
```
### Where new code would go
- `atr/models/sql.py` — in ReleasePhase enum
Add ARCHIVED = 'archived' to the ReleasePhase enum to represent archived
releases.
- `atr/admin/__init__.py` — after delete_release_post
Add archive_release_get and archive_release_post endpoints for archiving
finished releases.
- `atr/storage/writers/release.py` — after delete method in
CommitteeParticipant
Add an archive method that transitions a RELEASE to ARCHIVED phase.
### Proposed approach
The implementation requires three main changes:
1. **Add `ARCHIVED` phase**: Add `ARCHIVED = 'archived'` to
`sql.ReleasePhase` enum in `atr/models/sql.py`. This creates an explicit
terminal state for releases that have been announced and are no longer active.
2. **Guard against deletion of finished releases**: Modify
`CommitteeParticipant.delete` in `atr/storage/writers/release.py` to reject
deletion when the release is in `RELEASE` or `ARCHIVED` phase (unless an
override flag is provided for exceptional cases). Update the admin delete UI to
filter out or clearly mark released/archived releases as non-deletable.
3. **Add archive functionality**: Create an `archive` method on
`CommitteeParticipant` (or a similar writer class) that validates the release
is in `RELEASE` phase and transitions it to `ARCHIVED`. Add corresponding admin
routes (`archive_release_get`/`archive_release_post`) with appropriate UI.
Update the all-releases template to show the new phase badge.
### Suggested patches
#### `atr/storage/writers/release.py`
Add phase guard to prevent deletion of finished releases and add archive
method
````diff
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -101,6 +101,10 @@
async def delete(
self,
project_key: safe.ProjectKey,
version: safe.VersionKey,
phase: db.Opt[sql.ReleasePhase] = db.NOT_SET,
include_downloads: bool = True,
) -> str | None:
"""Handle the deletion of database records and filesystem data for
a release."""
release = await self.__data.release(
project_key=str(project_key), version=str(version),
phase=phase, _committee=True
).demand(storage.AccessError(f"Release '{project_key!s}
{version!s}' not found.", status=404))
+
+ # Finished or archived releases may not be deleted; they must be
archived instead
+ if release.phase in (sql.ReleasePhase.RELEASE,
sql.ReleasePhase.ARCHIVED):
+ raise storage.AccessError(
+ f"Release '{project_key!s} {version!s}' is in phase
'{release.phase.value}' and cannot be deleted. "
+ "Finished releases may only be archived.",
+ status=409,
+ )
+
release_dirs = [
paths.release_directory_base(release),
paths.get_attestable_dir() / str(project_key) / str(version),
````
#### `atr/storage/writers/release.py`
Add archive method to transition RELEASE phase to ARCHIVED
````diff
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -195,6 +195,33 @@
self.__write_as.append_to_audit_log(
asf_uid=self.__asf_uid,
project_key=str(project_key),
version=str(version),
error=error,
)
return error
+ async def archive(
+ self,
+ project_key: safe.ProjectKey,
+ version: safe.VersionKey,
+ ) -> str | None:
+ """Archive a finished release, transitioning it from RELEASE to
ARCHIVED phase."""
+ release = await self.__data.release(
+ project_key=str(project_key), version=str(version),
_committee=True
+ ).demand(storage.AccessError(f"Release '{project_key!s}
{version!s}' not found.", status=404))
+
+ if release.phase != sql.ReleasePhase.RELEASE:
+ raise storage.AccessError(
+ f"Only finished releases can be archived. "
+ f"Release '{project_key!s} {version!s}' is in phase
'{release.phase.value}'.",
+ status=409,
+ )
+
+ via = sql.validate_instrumented_attribute
+ stmt = sqlmodel.update(sql.Release).where(
+ via(sql.Release.key) == release.key,
+ via(sql.Release.phase) == sql.ReleasePhase.RELEASE,
+ ).values(phase=sql.ReleasePhase.ARCHIVED) # TODO: verify ARCHIVED
enum value exists
+ await self.__data.execute(stmt)
+ await self.__data.commit()
+
+ self.__write_as.append_to_audit_log(
+ asf_uid=self.__asf_uid,
+ project_key=str(project_key),
+ version=str(version),
+ action="archive",
+ )
+ return None
+
async def delete_empty_directory(
````
#### `atr/admin/templates/all-releases.html`
Add display for the ARCHIVED phase badge
````diff
--- a/atr/admin/templates/all-releases.html
+++ b/atr/admin/templates/all-releases.html
@@ -42,6 +42,8 @@
{% elif phase_value == "release" %}
<span class="badge bg-success">4. Release</span>
+ {% elif phase_value == "archived" %}
+ <span class="badge bg-dark">5. Archived</span>
{% else %}
<span class="badge bg-dark">Unknown</span>
{% endif %}
````
#### `atr/admin/__init__.py`
Filter admin delete form to exclude finished/archived releases that should
only be archived
````diff
--- a/atr/admin/__init__.py
+++ b/atr/admin/__init__.py
@@ -240,7 +240,12 @@
async def delete_release_get(
_session: web.Committer, _delete_release_get: Literal["delete-release"]
) -> str | web.WerkzeugResponse:
"""
URL: GET /delete-release
Display the form to delete releases.
"""
async with db.session() as data:
releases = await
data.release(_project=True).order_by(sql.Release.key).all()
+ # Finished and archived releases cannot be deleted, only archived
+ NON_DELETABLE_PHASES = {sql.ReleasePhase.RELEASE,
sql.ReleasePhase.ARCHIVED} # TODO: verify ARCHIVED exists
+ deletable_releases = [r for r in releases if r.phase not in
NON_DELETABLE_PHASES]
+ non_deletable_releases = [r for r in releases if r.phase in
NON_DELETABLE_PHASES]
+
- if releases:
+ if deletable_releases:
releases_widget = htpy.div[
[
htpy.div(".form-check")[
htpy.input(
class_="form-check-input",
type="checkbox",
name="releases_to_delete",
value=release.key,
id=f"release_{release.key}",
),
htpy.label(".form-check-label",
for_=f"release_{release.key}")[
htpy.strong[release.key],
f" ({release.project.display_name},
{release.phase.value.upper()})",
],
]
- for release in releases
+ for release in deletable_releases
]
]
````
### Open questions
- The `ReleasePhase` enum in `atr/models/sql.py` was not provided - need to
confirm the exact enum definition and add `ARCHIVED = 'archived'` there.
- Should there be a database migration for the new enum value, or does
SQLite handle this transparently?
- Should archived releases still be visible in public release listings
(`atr/get/release.py`), or should they be filtered out?
- Does the `start` method in `CommitteeParticipant` need to handle the case
where a previous ARCHIVED release exists for the same version (to allow
re-releasing)?
- Should the archive action be restricted to PMC members only
(CommitteeMember) rather than CommitteeParticipant?
- The admin `_delete_releases` helper function (not fully visible) may need
similar phase guards.
### Files examined
- `atr/storage/writers/release.py`
- `atr/admin/templates/delete-release.html`
- `atr/admin/__init__.py`
- `atr/blueprints/admin.py`
- `atr/storage/writers/revision.py`
- `atr/admin/templates/all-releases.html`
- `atr/post/draft.py`
- `atr/get/release.py`
### Related issues
This issue appears related to: #510.
_Both address constraints on deletion and archival of releases and projects_
---
*Draft from a triage agent. A human reviewer should validate before merging
any change. The agent did not run tests or verify diffs apply.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
