You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tooling.apache.org by "alitheg (via GitHub)" <gi...@apache.org> on 2026/05/27 08:52:18 UTC
[GH] Warn about and then delete inactive unfinished releases (tooling-trusted-releases)
alitheg commented on code in PR #1271:
URL: https://github.com/apache/tooling-trusted-releases/pull/1271#discussion_r3309658261
##########
atr/storage/writers/release.py:
##########
@@ -1170,3 +1254,205 @@ def __init__(self, write: storage.Write, write_as: storage.WriteAsFoundationAdmi
if asf_uid is None:
raise storage.AccessError("Not authorized", status=403)
self.__asf_uid = asf_uid
+
+ async def delete_inactive(
+ self,
+ project_key: safe.ProjectKey,
+ version: safe.VersionKey,
+ dry_run: bool = False,
+ ) -> str | None:
+ error = await self.__check_eligibility(project_key, version)
+ if error is not None:
+ return error
+ if dry_run:
+ return None
+ await self.__data.begin_immediate()
+ self.__data.expire_all()
+ final_error = await self.__check_eligibility(project_key, version)
+ if final_error is not None:
+ await self.__data.rollback()
+ return final_error
+ return await self.__delete(project_key, version)
+
+ async def __delete(
+ self,
+ project_key: safe.ProjectKey,
+ version: safe.VersionKey,
+ include_downloads: bool = True,
+ ) -> str | None:
+ 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)
+ )
+ storage.ensure_project_active(release.project)
+ return await self.__delete_body(release, project_key, version, include_downloads)
+
+ async def __delete_body(
+ self,
+ release: sql.Release,
+ project_key: safe.ProjectKey,
+ version: safe.VersionKey,
+ include_downloads: bool,
+ ) -> str | None:
+ release_dirs = [
+ paths.release_directory_base(release),
+ paths.get_attestable_dir() / str(project_key) / str(version),
+ paths.get_archives_dir() / str(project_key) / str(version),
+ paths.get_quarantined_dir() / str(project_key) / str(version),
+ ]
+
+ # Delete from the database using bulk SQL DELETE for efficiency
+ log.info(f"Deleting database records for release: {project_key!s} {version!s}")
+
+ # Bulk delete tasks
+ # These is no cascade, so we must delete explicitly
+ via = sql.validate_instrumented_attribute
+ task_delete_stmt = sqlmodel.delete(sql.Task).where(
+ via(sql.Task.project_key) == release.project.key,
+ via(sql.Task.version_key) == release.version,
+ )
+ task_result = await self.__data.execute(task_delete_stmt)
+ task_count = task_result.rowcount if isinstance(task_result, engine.CursorResult) else 0
+ log.debug(f"Deleted {util.plural(task_count, 'task')} for {project_key!s} {version!s}")
+
+ release_key = release.key
+
+ # These deletes would also be performed by database cascade
+ # We do them here before the commit instead to be explicit
+ rfs_delete_stmt = sqlmodel.delete(sql.ReleaseFileState).where(
+ via(sql.ReleaseFileState.release_key) == release_key,
+ )
+ rfs_result = await self.__data.execute(rfs_delete_stmt)
+ rfs_count = rfs_result.rowcount if isinstance(rfs_result, engine.CursorResult) else 0
+ log.debug(f"Deleted {util.plural(rfs_count, 'file state row')} for {project_key!s} {version!s}")
+
+ await self.__data.delete(release)
+ log.info(f"Deleted release record: {project_key!s} {version!s}")
+
+ # In test mode, delete the counter for test committee releases
+ # This allows revision numbers to be reused in testing
+ committee = release.project.committee
+ is_test_release = config.is_test_mode() and (committee is not None) and (committee.key == "test")
+ if is_test_release:
+ counter_delete_stmt = sqlmodel.delete(sql.RevisionCounter).where(
+ via(sql.RevisionCounter.release_key) == release_key
+ )
+ await self.__data.execute(counter_delete_stmt)
+ vote_counter_delete_stmt = sqlmodel.delete(sql.VoteCounter).where(
+ via(sql.VoteCounter.release_key) == release_key
+ )
+ await self.__data.execute(vote_counter_delete_stmt)
+ log.info(f"Deleted revision and vote counters for test release: {release_key}")
+
+ # Filesystem deletions are more likely to have permission errors than database deletions
+ # Therefore we do filesystem deletions first
+ if include_downloads:
+ await self.__delete_release_data_downloads(release)
+ error = await self.__delete_release_data_filesystem(release_dirs, project_key, version)
+
+ await self.__data.commit()
+
+ 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 __delete_release_data_downloads(self, release: sql.Release) -> None:
+ finished_dir = paths.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 = paths.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)
+ log.info(f"Deleted hard link: {full_link_path}")
+ except FileNotFoundError:
+ continue
+
+ async def __delete_release_data_filesystem(
+ self, release_dirs: Sequence[safe.StatePath], project_key: safe.ProjectKey, version: safe.VersionKey
+ ) -> str | None:
+ delete_errors: list[str] = []
+ for release_dir in release_dirs:
+ if await aiofiles.os.path.isdir(release_dir):
+ try:
+ log.info(f"Deleting filesystem directory: {release_dir}")
+ await util.delete_immutable_directory(
+ release_dir, reason=f"user {self.__asf_uid} is deleting release {project_key!s} {version!s}"
+ )
+ log.info(f"Successfully deleted directory: {release_dir}")
+ except Exception as e:
+ log.exception(f"Error deleting filesystem directory {release_dir}:")
+ delete_errors.append(f"{release_dir}: {e!s}")
+ else:
+ log.warning(f"Filesystem directory not found, skipping deletion: {release_dir}")
+ if delete_errors:
+ return (
+ f"Database records for '{project_key!s} {version!s}' deleted,"
+ f" but failed to delete filesystem directories: {', '.join(delete_errors)}"
+ )
+ return None
+
+ async def __check_eligibility(
+ self,
+ project_key: safe.ProjectKey,
+ version: safe.VersionKey,
+ ) -> str | None:
+ release = await self.__data.release(project_key=str(project_key), version=str(version), _committee=True).get()
+ if release is None:
+ return "release no longer exists"
+ if release.project.status != sql.ProjectStatus.ACTIVE:
Review Comment:
I've been wondering if we should add a property onto project called "is_active" rather than repeating this check everywhere. What do you think?
--
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: dev-unsubscribe@tooling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For additional commands, e-mail: dev-help@tooling.apache.org