You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by sb...@apache.org on 2026/06/09 15:33:23 UTC
(tooling-trusted-releases) 01/01: Make deleting inactive releases more resilient to failures
This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch sbp
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
commit 71ded7f682675de901c3cc249898483752ab5080
Author: Sean B. Palmer <se...@miscoranda.com>
AuthorDate: Tue Jun 9 16:32:30 2026 +0100
Make deleting inactive releases more resilient to failures
---
atr/storage/writers/release.py | 116 +++++++++++++++++-------------
tests/unit/test_release_writer_archive.py | 92 ++++++++++++++++--------
2 files changed, 130 insertions(+), 78 deletions(-)
diff --git a/atr/storage/writers/release.py b/atr/storage/writers/release.py
index c9203ef2..03666c78 100644
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -307,27 +307,34 @@ class CommitteeParticipant(FoundationCommitter):
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
+ try:
+ await self.__release_download_links_delete(release)
+ except Exception:
+ log.exception(f"Error removing download hard links for release {release.key!r}; continuing with deletion")
- 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 __release_download_links_delete(self, release: sql.Release) -> None:
+ finished_dir = paths.release_directory(release)
+ if not await aiofiles.os.path.isdir(finished_dir):
+ return
+ release_inodes: set[int] = set()
+ async for file_path in util.paths_recursive_all(finished_dir):
+ try:
+ stat_result = await aiofiles.os.stat(finished_dir / file_path)
+ except OSError:
+ continue
+ release_inodes.add(stat_result.st_ino)
+ if not release_inodes:
+ return
+ downloads_dir = paths.get_downloads_dir()
+ async for link_path in util.paths_recursive_all(downloads_dir):
+ full_link_path = downloads_dir / link_path
+ try:
+ link_stat = await aiofiles.os.stat(full_link_path)
+ except OSError:
+ continue
+ if link_stat.st_ino in release_inodes:
+ await aiofiles.os.remove(full_link_path)
+ log.info(f"Deleted hard link: {full_link_path}")
async def __delete_release_data_filesystem(
self, release_dirs: Sequence[safe.StatePath], project_key: safe.ProjectKey, version: safe.VersionKey
@@ -1345,6 +1352,12 @@ class CommitteeMember(CommitteeParticipant):
return None
async def __remove_from_downloads(self, release: sql.Release) -> None:
+ try:
+ await self.__release_download_links_delete(release)
+ except Exception:
+ log.exception(f"Error removing download hard links for release {release.key!r}; continuing with archive")
+
+ async def __release_download_links_delete(self, release: sql.Release) -> None:
# Hard-linked downloads share an inode with the finished files, so we
# find them by inode rather than path - that way an unknown
# download_path_suffix isn't a blocker.
@@ -1352,24 +1365,24 @@ class CommitteeMember(CommitteeParticipant):
if not await aiofiles.os.path.isdir(finished_dir):
return
release_inodes: set[int] = set()
- async for file_path in util.paths_recursive(finished_dir):
+ async for file_path in util.paths_recursive_all(finished_dir):
try:
stat_result = await aiofiles.os.stat(finished_dir / file_path)
- release_inodes.add(stat_result.st_ino)
- except FileNotFoundError:
+ except OSError:
continue
+ release_inodes.add(stat_result.st_ino)
if not release_inodes:
return
downloads_dir = paths.get_downloads_dir()
- async for link_path in util.paths_recursive(downloads_dir):
+ async for link_path in util.paths_recursive_all(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 download hard link: {full_link_path}")
- except FileNotFoundError:
+ except OSError:
continue
+ if link_stat.st_ino in release_inodes:
+ await aiofiles.os.remove(full_link_path)
+ log.info(f"Deleted download hard link: {full_link_path}")
class FoundationAdmin(FoundationCommitter):
@@ -1501,27 +1514,34 @@ class FoundationAdmin(FoundationCommitter):
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
+ try:
+ await self.__release_download_links_delete(release)
+ except Exception:
+ log.exception(f"Error removing download hard links for release {release.key!r}; continuing with deletion")
- 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 __release_download_links_delete(self, release: sql.Release) -> None:
+ finished_dir = paths.release_directory(release)
+ if not await aiofiles.os.path.isdir(finished_dir):
+ return
+ release_inodes: set[int] = set()
+ async for file_path in util.paths_recursive_all(finished_dir):
+ try:
+ stat_result = await aiofiles.os.stat(finished_dir / file_path)
+ except OSError:
+ continue
+ release_inodes.add(stat_result.st_ino)
+ if not release_inodes:
+ return
+ downloads_dir = paths.get_downloads_dir()
+ async for link_path in util.paths_recursive_all(downloads_dir):
+ full_link_path = downloads_dir / link_path
+ try:
+ link_stat = await aiofiles.os.stat(full_link_path)
+ except OSError:
+ continue
+ if link_stat.st_ino in release_inodes:
+ await aiofiles.os.remove(full_link_path)
+ log.info(f"Deleted hard link: {full_link_path}")
async def __delete_release_data_filesystem(
self, release_dirs: Sequence[safe.StatePath], project_key: safe.ProjectKey, version: safe.VersionKey
diff --git a/tests/unit/test_release_writer_archive.py b/tests/unit/test_release_writer_archive.py
index 0e3d6b2d..401fd47a 100644
--- a/tests/unit/test_release_writer_archive.py
+++ b/tests/unit/test_release_writer_archive.py
@@ -16,6 +16,7 @@
# under the License.
import datetime
+import os
import unittest.mock as mock
from types import SimpleNamespace
@@ -34,21 +35,19 @@ class _ReleaseQuery:
return self._result
-def _make_member(release_result: object) -> release.CommitteeMember:
- mock_data = mock.MagicMock()
- mock_data.release = mock.MagicMock(return_value=_ReleaseQuery(release_result))
- mock_data.execute_query = mock.AsyncMock()
- mock_data.add = mock.MagicMock()
- mock_data.commit = mock.AsyncMock()
-
- mock_write = mock.MagicMock()
- mock_write.authorisation.asf_uid = "alice"
- mock_write_as = mock.MagicMock()
-
- member = release.CommitteeMember(mock_write, mock_write_as, mock_data, "test")
- # Skip the filesystem-side cleanup - covered by integration tests.
- object.__setattr__(member, "_CommitteeMember__remove_from_downloads", mock.AsyncMock())
- return member
+@pytest.mark.asyncio
+async def test_archive_returns_error_when_release_already_archived():
+ fake_release = SimpleNamespace(
+ key="example-1.0.0",
+ phase=sql.ReleasePhase.RELEASE,
+ archived=datetime.datetime(2026, 1, 1, tzinfo=datetime.UTC),
+ project_key="example",
+ cycle_key="example-default",
+ )
+ member = _make_member(release_result=fake_release)
+ error = await member.archive(safe.ProjectKey("example"), safe.VersionKey("1.0.0"))
+ assert error is not None
+ assert "already archived" in error
@pytest.mark.asyncio
@@ -74,21 +73,6 @@ async def test_archive_returns_error_when_release_not_in_release_phase():
assert "not in the release phase" in error
-@pytest.mark.asyncio
-async def test_archive_returns_error_when_release_already_archived():
- fake_release = SimpleNamespace(
- key="example-1.0.0",
- phase=sql.ReleasePhase.RELEASE,
- archived=datetime.datetime(2026, 1, 1, tzinfo=datetime.UTC),
- project_key="example",
- cycle_key="example-default",
- )
- member = _make_member(release_result=fake_release)
- error = await member.archive(safe.ProjectKey("example"), safe.VersionKey("1.0.0"))
- assert error is not None
- assert "already archived" in error
-
-
@pytest.mark.asyncio
async def test_archive_succeeds_and_writes_lifecycle_event():
fake_release = SimpleNamespace(
@@ -121,3 +105,51 @@ async def test_archive_succeeds_and_writes_lifecycle_event():
assert mock_data.execute_query.await_count == 1
# Commit happened
assert mock_data.commit.await_count == 1
+
+
+@pytest.mark.asyncio
+async def test_download_links_delete_tolerates_unsafe_filenames(tmp_path, monkeypatch):
+ finished_dir = tmp_path / "finished"
+ finished_dir.mkdir()
+ bad_file = finished_dir / "ELC Admin Guide.pdf"
+ bad_file.write_text("data")
+
+ downloads_dir = tmp_path / "downloads"
+ downloads_dir.mkdir()
+ link_path = downloads_dir / "ELC Admin Guide.pdf"
+ os.link(bad_file, link_path)
+
+ monkeypatch.setattr(release.paths, "release_directory", lambda _release: finished_dir)
+ monkeypatch.setattr(release.paths, "get_downloads_dir", lambda: downloads_dir)
+
+ member = _make_member(release_result=None)
+ await member._CommitteeMember__release_download_links_delete(SimpleNamespace(key="example-1.0.0"))
+
+ assert not link_path.exists()
+ assert bad_file.exists()
+
+
+@pytest.mark.asyncio
+async def test_remove_from_downloads_swallows_errors():
+ member = release.CommitteeMember(mock.MagicMock(), mock.MagicMock(), mock.MagicMock(), "test")
+ boom = mock.AsyncMock(side_effect=RuntimeError("boom"))
+ object.__setattr__(member, "_CommitteeMember__release_download_links_delete", boom)
+ await member._CommitteeMember__remove_from_downloads(SimpleNamespace(key="example-1.0.0"))
+ boom.assert_awaited_once()
+
+
+def _make_member(release_result: object) -> release.CommitteeMember:
+ mock_data = mock.MagicMock()
+ mock_data.release = mock.MagicMock(return_value=_ReleaseQuery(release_result))
+ mock_data.execute_query = mock.AsyncMock()
+ mock_data.add = mock.MagicMock()
+ mock_data.commit = mock.AsyncMock()
+
+ mock_write = mock.MagicMock()
+ mock_write.authorisation.asf_uid = "alice"
+ mock_write_as = mock.MagicMock()
+
+ member = release.CommitteeMember(mock_write, mock_write_as, mock_data, "test")
+ # Skip the filesystem-side cleanup - covered by integration tests.
+ object.__setattr__(member, "_CommitteeMember__remove_from_downloads", mock.AsyncMock())
+ return member
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org