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
The following commit(s) were added to refs/heads/sbp by this push:
new 3799e8e6 Use two separate functions for the main phases of revision
finalisation
3799e8e6 is described below
commit 3799e8e6006db477516d5bf62a7bcc9a20498492
Author: Sean B. Palmer <[email protected]>
AuthorDate: Mon Mar 2 19:20:58 2026 +0000
Use two separate functions for the main phases of revision finalisation
---
atr/storage/writers/revision.py | 134 +++++++++++++++++++++++++++++--------
tests/unit/test_create_revision.py | 13 +++-
2 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/atr/storage/writers/revision.py b/atr/storage/writers/revision.py
index ac67099d..a2fbf3ac 100644
--- a/atr/storage/writers/revision.py
+++ b/atr/storage/writers/revision.py
@@ -86,6 +86,58 @@ async def finalise_revision(
temp_dir_path: pathlib.Path,
version_name: str,
was_quarantined: bool = False,
+) -> sql.Revision:
+ try:
+ previous_attestable, _, merged_release = await _lock_and_merge(
+ data,
+ base_hashes=base_hashes,
+ base_inodes=base_inodes,
+ merge_enabled=merge_enabled,
+ n_inodes=n_inodes,
+ old_revision=old_revision,
+ path_to_hash=path_to_hash,
+ path_to_size=path_to_size,
+ previous_attestable=previous_attestable,
+ project_name=project_name,
+ release=release,
+ _release_name=release_name,
+ temp_dir_path=temp_dir_path,
+ version_name=version_name,
+ )
+ except Exception:
+ await aioshutil.rmtree(temp_dir)
+ raise
+
+ return await _commit_new_revision(
+ data,
+ asf_uid=asf_uid,
+ description=description,
+ path_to_hash=path_to_hash,
+ path_to_size=path_to_size,
+ previous_attestable=previous_attestable,
+ project_name=project_name,
+ release=merged_release,
+ release_name=release_name,
+ temp_dir=temp_dir,
+ version_name=version_name,
+ was_quarantined=was_quarantined,
+ )
+
+
+async def _commit_new_revision(
+ data: db.Session,
+ *,
+ asf_uid: str,
+ description: str | None,
+ path_to_hash: dict[str, str],
+ path_to_size: dict[str, int],
+ previous_attestable: atr.models.attestable.AttestableV1 | None,
+ project_name: str,
+ release: sql.Release,
+ release_name: str,
+ temp_dir: str,
+ version_name: str,
+ was_quarantined: bool = False,
) -> sql.Revision:
try:
# This is the only place where models.Revision is constructed
@@ -101,41 +153,12 @@ async def finalise_revision(
description=description,
was_quarantined=was_quarantined,
)
-
- # Acquire the write lock and add the row
- # We need this write lock for moving the directory below atomically
- # But it also helps to make models.populate_revision_sequence_and_name
safe against races
- await data.begin_immediate()
data.add(new_revision)
# Flush but do not commit the new revision row to get its name and
number
# The row will still be invisible to other sessions after flushing
await data.flush()
- # Merge with the prior revision if there was an intervening change
- prior_name = new_revision.parent_name
- if (
- merge_enabled
- and (old_revision is not None)
- and (prior_name is not None)
- and (prior_name != old_revision.name)
- ):
- prior_number = prior_name.split()[-1]
- prior_dir = paths.release_directory_base(release) / prior_number
- await merge.merge(
- base_inodes,
- base_hashes,
- prior_dir,
- project_name,
- version_name,
- prior_number,
- temp_dir_path,
- n_inodes,
- path_to_hash,
- path_to_size,
- )
- previous_attestable = await attestable.load(project_name,
version_name, prior_number)
-
# Rename the directory to the new revision number
await data.refresh(release)
new_revision_dir = paths.release_directory(release)
@@ -191,6 +214,59 @@ async def finalise_revision(
return new_revision
+async def _lock_and_merge(
+ data: db.Session,
+ *,
+ base_hashes: dict[str, str],
+ base_inodes: dict[str, int],
+ merge_enabled: bool,
+ n_inodes: dict[str, int],
+ old_revision: sql.Revision | None,
+ path_to_hash: dict[str, str],
+ path_to_size: dict[str, int],
+ previous_attestable: atr.models.attestable.AttestableV1 | None,
+ project_name: str,
+ release: sql.Release,
+ _release_name: str,
+ temp_dir_path: pathlib.Path,
+ version_name: str,
+) -> tuple[atr.models.attestable.AttestableV1 | None, str | None, sql.Release]:
+ # Acquire the write lock
+ # We need this write lock for moving the directory afterwards atomically
+ # But it also helps to make models.populate_revision_sequence_and_name
safe against races
+ await data.begin_immediate()
+
+ merged_release: sql.Release = await data.merge(release)
+ await data.refresh(merged_release)
+ latest = await interaction.latest_revision(merged_release,
caller_data=data)
+ prior_revision_name = latest.name if latest else None
+
+ # Merge with the prior revision if there was an intervening change
+ if (
+ merge_enabled
+ and (old_revision is not None)
+ and (prior_revision_name is not None)
+ and (prior_revision_name != old_revision.name)
+ ):
+ prior_number = prior_revision_name.split()[-1]
+ prior_dir = paths.release_directory_base(merged_release) / prior_number
+ await merge.merge(
+ base_inodes,
+ base_hashes,
+ prior_dir,
+ project_name,
+ version_name,
+ prior_number,
+ temp_dir_path,
+ n_inodes,
+ path_to_hash,
+ path_to_size,
+ )
+ previous_attestable = await attestable.load(project_name,
version_name, prior_number)
+
+ return previous_attestable, prior_revision_name, merged_release
+
+
class GeneralPublic:
def __init__(
self,
diff --git a/tests/unit/test_create_revision.py
b/tests/unit/test_create_revision.py
index a38e9090..13f84a66 100644
--- a/tests/unit/test_create_revision.py
+++ b/tests/unit/test_create_revision.py
@@ -67,6 +67,7 @@ class MockSafeData:
self.begin_immediate = mock.AsyncMock()
self.commit = mock.AsyncMock()
self.flush = mock.AsyncMock(side_effect=self._flush)
+ self.merge = mock.AsyncMock(side_effect=self._merge)
self.refresh = mock.AsyncMock()
def _add(self, new_revision: "FakeRevision") -> None:
@@ -79,6 +80,9 @@ class MockSafeData:
self._new_revision.number = self._new_number
self._new_revision.parent_name = self._parent_name
+ async def _merge(self, obj: object) -> object:
+ return obj
+
class MockSafeSession:
def __init__(self, data: MockSafeData):
@@ -187,6 +191,10 @@ async def
test_intervening_revision_triggers_merge_and_uses_latest_parent(tmp_pa
old_revision.name = f"{release_name} 00005"
old_revision.number = "00005"
+ intervening_revision = mock.MagicMock()
+ intervening_revision.name = f"{release_name} 00006"
+ intervening_revision.number = "00006"
+
first_attestable = mock.MagicMock(paths={"dist/a.tar.gz": "h1"})
second_attestable = mock.MagicMock(paths={"dist/b.tar.gz": "h2"})
@@ -207,7 +215,10 @@ async def
test_intervening_revision_triggers_merge_and_uses_latest_parent(tmp_pa
mock.patch.object(revision.db, "session", return_value=mock_session),
mock.patch.object(revision.detection, "validate_directory",
return_value=[]),
mock.patch.object(
- revision.interaction, "latest_revision",
new_callable=mock.AsyncMock, return_value=old_revision
+ revision.interaction,
+ "latest_revision",
+ new_callable=mock.AsyncMock,
+ side_effect=[old_revision, intervening_revision],
),
mock.patch.object(revision.merge, "merge", new=merge_mock),
mock.patch.object(revision.sql, "Revision",
side_effect=_make_fake_revision),
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
