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-releases.git
The following commit(s) were added to refs/heads/main by this push:
new 3716e77 Use the revision counter when creating new revisions
3716e77 is described below
commit 3716e774af9c85ae6476b62bd063ff1285b4a866
Author: Sean B. Palmer <[email protected]>
AuthorDate: Wed Jan 14 17:00:53 2026 +0000
Use the revision counter when creating new revisions
---
atr/models/sql.py | 57 +++++++++++++------------
atr/storage/writers/release.py | 21 ++++++---
migrations/versions/0039_2026.01.14_cd44f0ea.py | 31 ++++++++++++++
3 files changed, 74 insertions(+), 35 deletions(-)
diff --git a/atr/models/sql.py b/atr/models/sql.py
index 495e36c..296c65d 100644
--- a/atr/models/sql.py
+++ b/atr/models/sql.py
@@ -28,6 +28,7 @@ from typing import Any, Final, Literal, Optional, TypeVar
import pydantic
import sqlalchemy
+import sqlalchemy.dialects.sqlite as sqlite
import sqlalchemy.event as event
import sqlalchemy.orm as orm
import sqlalchemy.sql.expression as expression
@@ -1190,7 +1191,7 @@ def revision_name(release_name: str, number: str) -> str:
@event.listens_for(Revision, "before_insert")
def populate_revision_sequence_and_name(
- mapper: orm.Mapper, connection: sqlalchemy.engine.Connection, revision:
Revision
+ _mapper: orm.Mapper, connection: sqlalchemy.engine.Connection, revision:
Revision
) -> None:
# We require Revision.release_name to have been set
if not revision.release_name:
@@ -1198,36 +1199,36 @@ def populate_revision_sequence_and_name(
# Otherwise, Revision.name would be "", Revision.seq 0, and
Revision.number ""
raise RuntimeError("Cannot populate revision sequence and name without
release_name")
- # Get the Revision with the maximum existing Revision.seq and the same
Revision.release_name
- stmt = (
- sqlmodel.select(Revision.seq, Revision.name)
- .where(Revision.release_name == revision.release_name)
+ # Allocate the next sequence number from the counter table
+ # This ensures that sequence numbers are never reused, even after release
deletion
+ # Uses ON CONFLICT DO UPDATE with RETURNING
+ upsert_stmt = (
+ sqlite.insert(RevisionCounter)
+ .values(release_name=revision.release_name, last_allocated_number=1)
+ .on_conflict_do_update(
+ index_elements=["release_name"],
+ set_={"last_allocated_number":
sqlalchemy.text("last_allocated_number + 1")},
+ )
+ .returning(sqlalchemy.literal_column("last_allocated_number"))
+ )
+ result = connection.execute(upsert_stmt)
+ new_seq = result.scalar_one()
+
+ revision.seq = new_seq
+ revision.number = str(new_seq).zfill(5)
+ revision.name = revision_name(revision.release_name, revision.number)
+
+ # Find the actual parent for the parent_name foreign key
+ # We cannot assume that the parent exists
+ parent_stmt = (
+ sqlmodel.select(validate_instrumented_attribute(Revision.name))
+ .where(validate_instrumented_attribute(Revision.release_name) ==
revision.release_name)
.order_by(sqlalchemy.desc(validate_instrumented_attribute(Revision.seq)))
.limit(1)
)
- parent_row = connection.execute(stmt).fetchone()
-
- # We cannot happy path this, because we must recalculate the Revision.name
afterwards
- if parent_row is None:
- # This is the first Revision for this Revision.release_name
- # Revision.seq is 0, but we use a 1-based system
- revision.seq = 1
- revision.number = str(revision.seq).zfill(5)
- else:
- # We don't have the ORM available in this event listener
- # Therefore we must construct a new Revision object from the database
row
- parent_row_seq = parent_row.seq
- parent_row_name = parent_row.name
- # Compute the next sequence number
- revision.seq = parent_row_seq + 1
- revision.number = str(revision.seq).zfill(5)
- # Set the parent_name foreign key. SQLAlchemy will handle the
relationship.
- revision.parent_name = parent_row_name
- # Do NOT set revision.parent directly here
-
- # Recalculate the Revision.name
- # This field has a unique constraint, which eliminates the potential for
race conditions
- revision.name = revision_name(revision.release_name, revision.number)
+ parent_row = connection.execute(parent_stmt).fetchone()
+ if parent_row is not None:
+ revision.parent_name = parent_row[0]
@event.listens_for(Release, "before_insert")
diff --git a/atr/storage/writers/release.py b/atr/storage/writers/release.py
index b8531c6..dd17174 100644
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -112,7 +112,7 @@ class CommitteeParticipant(FoundationCommitter):
) -> str | None:
"""Handle the deletion of database records and filesystem data for a
release."""
release = await self.__data.release(
- project_name=project_name, version=version, phase=phase,
_project=True
+ project_name=project_name, version=version, phase=phase,
_committee=True
).demand(storage.AccessError(f"Release '{project_name} {version}' not
found."))
release_dir = util.release_directory_base(release)
@@ -139,14 +139,21 @@ class CommitteeParticipant(FoundationCommitter):
check_count = check_result.rowcount if isinstance(check_result,
engine.CursorResult) else 0
log.debug(f"Deleted {util.plural(check_count, 'check result')} for
{project_name} {version}")
- # TODO: Ensure that revisions are not deleted
- # But this makes testing difficult
- # Perhaps delete revisions if associated with test accounts only
- # But we want to test actual mechanisms, not special case tests
- # We could create uniquely named releases in tests
- # Currently part of the discussion in #171, but should be its own issue
+ release_name = release.name
await self.__data.delete(release)
log.info(f"Deleted release record: {project_name} {version}")
+
+ # 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.get().ALLOW_TESTS and (committee is not None)
and (committee.name == "test")
+ if is_test_release:
+ counter_delete_stmt = sqlmodel.delete(sql.RevisionCounter).where(
+ via(sql.RevisionCounter.release_name) == release_name
+ )
+ await self.__data.execute(counter_delete_stmt)
+ log.info(f"Deleted revision counter for test release:
{release_name}")
+
await self.__data.commit()
if include_downloads:
diff --git a/migrations/versions/0039_2026.01.14_cd44f0ea.py
b/migrations/versions/0039_2026.01.14_cd44f0ea.py
new file mode 100644
index 0000000..4c1bbfa
--- /dev/null
+++ b/migrations/versions/0039_2026.01.14_cd44f0ea.py
@@ -0,0 +1,31 @@
+"""Use current RevisionCounter values and switch the allocation logic
+
+Revision ID: 0039_2026.01.14_cd44f0ea
+Revises: 0038_2026.01.14_267562c1
+Create Date: 2026-01-14 16:12:28.673361+00:00
+"""
+
+from collections.abc import Sequence
+
+from alembic import op
+
+revision: str = "0039_2026.01.14_cd44f0ea"
+down_revision: str | None = "0038_2026.01.14_267562c1"
+branch_labels: str | Sequence[str] | None = None
+depends_on: str | Sequence[str] | None = None
+
+
+def upgrade() -> None:
+ # Update RevisionCounter.last_allocated_number to max(seq) for each release
+ # This must be run before the new allocation logic can be used
+ op.execute("""
+ UPDATE revisioncounter
+ SET last_allocated_number = COALESCE(
+ (SELECT MAX(seq) FROM revision WHERE revision.release_name =
revisioncounter.release_name),
+ 0
+ )
+ """)
+
+
+def downgrade() -> None:
+ op.execute("UPDATE revisioncounter SET last_allocated_number = 0")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
