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 ac1e85f Delete drafts correctly
ac1e85f is described below
commit ac1e85fd37eea4b254b556f4aff1781aa30a8faa
Author: Sean B. Palmer <[email protected]>
AuthorDate: Fri Jul 11 15:48:44 2025 +0100
Delete drafts correctly
---
atr/blueprints/admin/admin.py | 75 +---------------------------------------
atr/blueprints/api/api.py | 8 +++--
atr/db/interaction.py | 80 +++++++++++++++++++++++++++++++++++++++++++
atr/revision.py | 2 +-
atr/routes/draft.py | 24 ++++---------
5 files changed, 94 insertions(+), 95 deletions(-)
diff --git a/atr/blueprints/admin/admin.py b/atr/blueprints/admin/admin.py
index 1c70819..c001530 100644
--- a/atr/blueprints/admin/admin.py
+++ b/atr/blueprints/admin/admin.py
@@ -343,7 +343,7 @@ async def admin_delete_release() -> str | response.Response:
for release_name in releases_to_delete:
try:
- await _delete_release_data(release_name)
+ await interaction.release_delete(release_name)
success_count += 1
except base.ASFQuartException as e:
_LOGGER.error("Error deleting release %s: %s",
release_name, e)
@@ -711,79 +711,6 @@ async def _check_keys(fix: bool = False) -> str:
return message
-async def _delete_release_data(release_name: str) -> None:
- """Handle the deletion of database records and filesystem data for a
release."""
- async with db.session() as data:
- release = await data.release(name=release_name, _project=True).demand(
- base.ASFQuartException(f"Release '{release_name}' not found.", 404)
- )
- release_dir = util.release_directory_base(release)
-
- # Delete from the database
- _LOGGER.info("Deleting database records for release: %s", release_name)
- # Cascade should handle this, but we delete manually anyway
- tasks_to_delete = await data.task(project_name=release.project.name,
version_name=release.version).all()
- for task in tasks_to_delete:
- await data.delete(task)
- _LOGGER.debug("Deleted %d tasks for %s", len(tasks_to_delete),
release_name)
-
- checks_to_delete = await
data.check_result(release_name=release_name).all()
- for check in checks_to_delete:
- await data.delete(check)
- _LOGGER.debug("Deleted %d check results for %s",
len(checks_to_delete), release_name)
-
- await data.delete(release)
- _LOGGER.info("Deleted release record: %s", release_name)
- await data.commit()
-
- await _delete_release_data_downloads(release)
- await _delete_release_data_filesystem(release_dir, release_name)
-
-
-async def _delete_release_data_downloads(release: models.Release) -> None:
- # Delete hard links from the downloads directory
- finished_dir = util.release_directory(release)
- if await aiofiles.os.path.isdir(finished_dir):
- release_inodes = set()
- async for file_path in util.paths_recursive(finished_dir):
- try:
- stat_result = await aiofiles.os.stat(finished_dir / file_path)
- release_inodes.add(stat_result.st_ino)
- except FileNotFoundError:
- continue
-
- if release_inodes:
- downloads_dir = util.get_downloads_dir()
- async for link_path in util.paths_recursive(downloads_dir):
- full_link_path = downloads_dir / link_path
- try:
- link_stat = await aiofiles.os.stat(full_link_path)
- if link_stat.st_ino in release_inodes:
- await aiofiles.os.remove(full_link_path)
- _LOGGER.info(f"Deleted hard link: {full_link_path}")
- except FileNotFoundError:
- continue
-
-
-async def _delete_release_data_filesystem(release_dir: pathlib.Path,
release_name: str) -> None:
- # Delete from the filesystem
- try:
- if await aiofiles.os.path.isdir(release_dir):
- _LOGGER.info("Deleting filesystem directory: %s", release_dir)
- # Believe this to be another bug in mypy Protocol handling
- # TODO: Confirm that this is a bug, and report upstream
- await aioshutil.rmtree(release_dir) # type: ignore[call-arg]
- _LOGGER.info("Successfully deleted directory: %s", release_dir)
- else:
- _LOGGER.warning("Filesystem directory not found, skipping
deletion: %s", release_dir)
- except Exception as e:
- _LOGGER.exception("Error deleting filesystem directory %s:",
release_dir)
- await quart.flash(
- f"Database records for '{release_name}' deleted, but failed to
delete filesystem directory: {e!s}",
- "warning",
- )
-
-
async def _get_filesystem_dirs() -> list[str]:
filesystem_dirs = []
await _get_filesystem_dirs_finished(filesystem_dirs)
diff --git a/atr/blueprints/api/api.py b/atr/blueprints/api/api.py
index 2be17c3..f1b10b1 100644
--- a/atr/blueprints/api/api.py
+++ b/atr/blueprints/api/api.py
@@ -34,11 +34,11 @@ import werkzeug.exceptions as exceptions
import atr.blueprints.api as api
import atr.db as db
+import atr.db.interaction as interaction
import atr.db.models as models
import atr.jwtoken as jwtoken
import atr.revision as revision
import atr.routes as routes
-import atr.routes.draft as draft
import atr.routes.start as start
import atr.routes.voting as voting
import atr.schema as schema
@@ -191,7 +191,11 @@ async def draft_delete_project_version() ->
tuple[dict[str, str], int]:
# TODO: This causes "A transaction is already begun on this Session"
# async with data.begin():
# Probably due to autobegin in data.release above
- await draft.delete_candidate_draft(data, release_name)
+ # We pass the phase again to guard against races
+ # But the removal is not actually locked
+ await interaction.release_delete(
+ release_name, phase=models.ReleasePhase.RELEASE_CANDIDATE_DRAFT,
include_downloads=False
+ )
await data.commit()
return {"deleted": release_name}, 200
diff --git a/atr/db/interaction.py b/atr/db/interaction.py
index 15c506c..a562181 100644
--- a/atr/db/interaction.py
+++ b/atr/db/interaction.py
@@ -25,6 +25,10 @@ import re
from collections.abc import AsyncGenerator
from typing import Final
+import aiofiles.os
+import aioshutil
+import asfquart.base as base
+import quart
import sqlalchemy
import sqlmodel
@@ -280,6 +284,38 @@ async def path_info(release: models.Release, paths:
list[pathlib.Path]) -> PathI
return info
+async def release_delete(
+ release_name: str, phase: db.Opt[models.ReleasePhase] = db.NOT_SET,
include_downloads: bool = True
+) -> None:
+ """Handle the deletion of database records and filesystem data for a
release."""
+ async with db.session() as data:
+ release = await data.release(name=release_name, phase=phase,
_project=True).demand(
+ base.ASFQuartException(f"Release '{release_name}' not found.", 404)
+ )
+ release_dir = util.release_directory_base(release)
+
+ # Delete from the database
+ _LOGGER.info("Deleting database records for release: %s", release_name)
+ # Cascade should handle this, but we delete manually anyway
+ tasks_to_delete = await data.task(project_name=release.project.name,
version_name=release.version).all()
+ for task in tasks_to_delete:
+ await data.delete(task)
+ _LOGGER.debug("Deleted %d tasks for %s", len(tasks_to_delete),
release_name)
+
+ checks_to_delete = await
data.check_result(release_name=release_name).all()
+ for check in checks_to_delete:
+ await data.delete(check)
+ _LOGGER.debug("Deleted %d check results for %s",
len(checks_to_delete), release_name)
+
+ await data.delete(release)
+ _LOGGER.info("Deleted release record: %s", release_name)
+ await data.commit()
+
+ if include_downloads:
+ await _delete_release_data_downloads(release)
+ await _delete_release_data_filesystem(release_dir, release_name)
+
+
async def tasks_ongoing(project_name: str, version_name: str, revision_number:
str) -> int:
async with db.session() as data:
query = (
@@ -389,6 +425,50 @@ async def upload_keys_bytes(
return results, success_count, error_count, submitted_committees
+async def _delete_release_data_downloads(release: models.Release) -> None:
+ # Delete hard links from the downloads directory
+ finished_dir = util.release_directory(release)
+ if await aiofiles.os.path.isdir(finished_dir):
+ release_inodes = set()
+ async for file_path in util.paths_recursive(finished_dir):
+ try:
+ stat_result = await aiofiles.os.stat(finished_dir / file_path)
+ release_inodes.add(stat_result.st_ino)
+ except FileNotFoundError:
+ continue
+
+ if release_inodes:
+ downloads_dir = util.get_downloads_dir()
+ async for link_path in util.paths_recursive(downloads_dir):
+ full_link_path = downloads_dir / link_path
+ try:
+ link_stat = await aiofiles.os.stat(full_link_path)
+ if link_stat.st_ino in release_inodes:
+ await aiofiles.os.remove(full_link_path)
+ _LOGGER.info(f"Deleted hard link: {full_link_path}")
+ except FileNotFoundError:
+ continue
+
+
+async def _delete_release_data_filesystem(release_dir: pathlib.Path,
release_name: str) -> None:
+ # Delete from the filesystem
+ try:
+ if await aiofiles.os.path.isdir(release_dir):
+ _LOGGER.info("Deleting filesystem directory: %s", release_dir)
+ # Believe this to be another bug in mypy Protocol handling
+ # TODO: Confirm that this is a bug, and report upstream
+ await aioshutil.rmtree(release_dir) # type: ignore[call-arg]
+ _LOGGER.info("Successfully deleted directory: %s", release_dir)
+ else:
+ _LOGGER.warning("Filesystem directory not found, skipping
deletion: %s", release_dir)
+ except Exception as e:
+ _LOGGER.exception("Error deleting filesystem directory %s:",
release_dir)
+ await quart.flash(
+ f"Database records for '{release_name}' deleted, but failed to
delete filesystem directory: {e!s}",
+ "warning",
+ )
+
+
def _key_latest_self_signature(key: dict) -> datetime.datetime | None:
fingerprint = key["fingerprint"]
# TODO: Only 64 bits, which is not at all secure
diff --git a/atr/revision.py b/atr/revision.py
index f9af9a5..cbe5529 100644
--- a/atr/revision.py
+++ b/atr/revision.py
@@ -86,7 +86,7 @@ async def create_and_manage(
# We ensure, below, that it's removed on any exception
# Use the tmp subdirectory of state, to ensure that it is on the same
filesystem
prefix_token = secrets.token_hex(16)
- temp_dir: str = await asyncio.to_thread(tempfile.mkdtemp,
prefix=prefix_token, dir=util.get_tmp_dir())
+ temp_dir: str = await asyncio.to_thread(tempfile.mkdtemp,
prefix=prefix_token + "-", dir=util.get_tmp_dir())
temp_dir_path = pathlib.Path(temp_dir)
creating = Creating(old=old_revision, interim_path=temp_dir_path,
new=None, failed=None)
try:
diff --git a/atr/routes/draft.py b/atr/routes/draft.py
index d3567f4..808ca77 100644
--- a/atr/routes/draft.py
+++ b/atr/routes/draft.py
@@ -24,7 +24,7 @@ import datetime
import hashlib
import logging
import pathlib
-from typing import TYPE_CHECKING, Final, Protocol, TypeVar
+from typing import TYPE_CHECKING, Protocol, TypeVar
import aiofiles.os
import aioshutil
@@ -35,6 +35,7 @@ import wtforms
import atr.analysis as analysis
import atr.construct as construct
import atr.db as db
+import atr.db.interaction as interaction
import atr.db.models as models
import atr.revision as revision
import atr.routes as routes
@@ -48,8 +49,7 @@ import atr.util as util
if TYPE_CHECKING:
import werkzeug.wrappers.response as response
-# _CONFIG: Final = config.get()
-_LOGGER: Final = logging.getLogger(__name__)
+# _LOGGER: Final = logging.getLogger(__name__)
T = TypeVar("T")
@@ -113,7 +113,9 @@ async def delete(session: routes.CommitterSession) ->
response.Response:
async with db.session() as data:
async with data.begin():
try:
- await delete_candidate_draft(data, release_name)
+ await interaction.release_delete(
+ release_name,
phase=models.ReleasePhase.RELEASE_CANDIDATE_DRAFT, include_downloads=False
+ )
except Exception as e:
logging.exception("Error deleting candidate draft:")
return await session.redirect(root.index, error=f"Error
deleting candidate draft: {e!s}")
@@ -131,20 +133,6 @@ async def delete(session: routes.CommitterSession) ->
response.Response:
return await session.redirect(root.index, success="Candidate draft deleted
successfully")
-async def delete_candidate_draft(data: db.Session, candidate_draft_name: str)
-> None:
- """Delete a candidate draft and all its associated files."""
- # Check that the release exists
- # TODO: Use session.release here
- release = await data.release(name=candidate_draft_name,
_project=True).get()
- if not release:
- raise routes.FlashError("Candidate draft not found")
- if release.phase != models.ReleasePhase.RELEASE_CANDIDATE_DRAFT:
- raise routes.FlashError("Candidate draft is not in the release
candidate draft phase")
-
- # Delete the release record
- await data.delete(release)
-
-
@routes.committer("/draft/delete-file/<project_name>/<version_name>",
methods=["POST"])
async def delete_file(session: routes.CommitterSession, project_name: str,
version_name: str) -> response.Response:
"""Delete a specific file from the release candidate, creating a new
revision."""
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
