You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tooling.apache.org by "sbp (via GitHub)" <gi...@apache.org> on 2026/04/20 14:47:00 UTC
[I] Disallow phase transitions from any but the latest revision (tooling-trusted-releases)
sbp opened a new issue, #1182:
URL: https://github.com/apache/tooling-trusted-releases/issues/1182
Currently we allow promoting compose to vote, and finish to release, on revisions which are not the latest. These older revisions don't have have database views of their files, so we can probably simplify the code by ensuring that these phase transitions (promotions) can only happen from the latest version. This would also make more sense from the point of view of consistency in the UI.
--
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.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
Re: [I] Disallow phase transitions from any but the latest revision (tooling-trusted-releases)
Posted by "asf-tooling (via GitHub)" <gi...@apache.org>.
asf-tooling commented on issue #1182:
URL: https://github.com/apache/tooling-trusted-releases/issues/1182#issuecomment-4409751023
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@2da7807a`
**Type:** `refactor` • **Classification:** `actionable` • **Confidence:** `medium`
**Application domain(s):** `release_lifecycle`
### Summary
The issue asks to disallow phase transitions from non-latest revisions. @sbp clarified this only applies to the compose→vote transition (finish no longer has revisions). The backend in `_start_vote_no_commit` already has a check rejecting non-latest revisions (raising a 409), but the `promote_to_candidate` method still accepts a `selected_revision_number` parameter, and presumably the UI still presents the option. The simplification would remove the ability to select a revision for promotion and always use the latest.
### Where this lives in the code today
#### `atr/storage/writers/release.py` — `CommitteeParticipant.promote_to_candidate` (lines 450-472)
_needs modification_
This method accepts a `selected_revision_number` parameter that the issue suggests removing in favor of always using the latest revision internally.
```python
async def promote_to_candidate(
self,
release_key: safe.ReleaseKey,
selected_revision_number: safe.RevisionNumber,
*,
allowed_vote_modes: frozenset[sql.VoteMode],
) -> str | None:
"""Promote a release candidate draft to a new phase."""
try:
await self.__data.begin_immediate()
release, vote_seq, vote_mode = await self._start_vote_no_commit(
release_key,
selected_revision_number,
allowed_vote_modes=allowed_vote_modes,
promote=True,
)
await self.__data.commit()
except storage.AccessError as e:
await self.__data.rollback()
return str(e)
except Exception:
await self.__data.rollback()
raise
```
#### `atr/storage/writers/release.py` — `CommitteeParticipant._start_vote_no_commit` (lines 483-503)
_currently does this_
This method already contains the enforcement check rejecting promotions from non-latest revisions. The issue's simplification would make this check unnecessary for the promote=True path by always using the latest revision internally.
```python
async def _start_vote_no_commit( # noqa: C901
self,
release_key: safe.ReleaseKey,
selected_revision_number: safe.RevisionNumber,
*,
allowed_vote_modes: frozenset[sql.VoteMode],
promote: bool,
expected_podling_thread_id: str | None = None,
) -> tuple[sql.Release, int, sql.VoteMode]:
release_for_pre_checks = await self.__data.release(
key=str(release_key), _project=True, _committee=True, _project_release_policy=True
).demand(storage.AccessError("Release candidate draft not found", status=404))
project_key = release_for_pre_checks.safe_project_key
version_key = release_for_pre_checks.safe_version_key
revision_number = release_for_pre_checks.safe_latest_revision_number
...
# Check that the revision number is the latest
if revision_number != selected_revision_number:
raise storage.AccessError(
"The selected revision number does not match the latest revision number", status=409
)
```
### Proposed approach
The simplification involves modifying `promote_to_candidate` to no longer accept a `selected_revision_number` parameter, instead internally looking up the latest revision number. This eliminates the need for the UI to pass a revision number and removes the possibility of even attempting to promote from a non-latest revision. The `_start_vote_no_commit` method would still receive the revision number for its optimistic concurrency check in the SQL UPDATE WHERE clause (preventing races), but it would derive it from `release_for_pre_checks.safe_latest_revision_number` rather than from user input when `promote=True`.
Since I cannot see the caller of `promote_to_candidate` (likely in a POST handler for vote/draft operations) or the template that renders the promote button, I cannot provide a complete diff. The core change is in `_start_vote_no_commit`: when `promote=True`, ignore the `selected_revision_number` parameter and use `revision_number` (the latest) directly. Then update callers to not need to pass a revision number for promotion.
### Suggested patches
#### `atr/storage/writers/release.py`
When promoting (compose→vote), always use the latest revision number instead of requiring the caller to supply it, simplifying the interface and preventing any possibility of non-latest promotion.
````diff
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -280,7 +280,7 @@
async def promote_to_candidate(
self,
release_key: safe.ReleaseKey,
- selected_revision_number: safe.RevisionNumber,
+ selected_revision_number: safe.RevisionNumber | None = None,
*,
allowed_vote_modes: frozenset[sql.VoteMode],
) -> str | None:
@@ -330,6 +330,12 @@
revision_number = release_for_pre_checks.safe_latest_revision_number
if promote:
vote_mode = release_for_pre_checks.effective_vote_mode
+ # When promoting, always use the latest revision - disallow promotion
+ # from any revision other than the latest
+ if selected_revision_number is None:
+ selected_revision_number = revision_number
+ elif revision_number != selected_revision_number:
+ raise storage.AccessError(
+ "Phase transitions are only allowed from the latest revision", status=409
+ )
else:
vote_mode = release_for_pre_checks.vote_mode
if vote_mode is None:
@@ -350,11 +356,6 @@
if promote:
raise storage.AccessError("This release is not in the candidate draft phase", status=409)
raise storage.AccessError("The release state has changed, please refresh and try again", status=409)
- # Check that the revision number is the latest
- if revision_number != selected_revision_number:
- raise storage.AccessError(
- "The selected revision number does not match the latest revision number", status=409
- )
if await interaction.has_blocker_checks(
release_for_pre_checks, selected_revision_number, caller_data=self.__data
````
### Open questions
- Where is `promote_to_candidate` called from? I don't see the POST handler that starts a vote (likely in `atr/post/draft.py` or similar). The caller also needs to be updated to stop passing a user-selected revision number.
- Is there a template that renders a revision selector for the promote/start-vote button? If so, that UI element should be removed or hidden.
- The `_start_vote_no_commit` method is also called with `promote=False` (for re-votes). Does the non-latest check need to be preserved for that path? Based on the issue, it seems only `promote=True` is affected.
- The SQL UPDATE uses `sql.latest_revision_number_query() == str(selected_revision_number)` as an optimistic concurrency guard. This should be kept regardless, using the internally-derived latest revision number.
### Files examined
- `atr/post/compose.py`
- `atr/post/finish.py`
- `atr/storage/writers/release.py`
- `atr/storage/writers/revision.py`
- `atr/post/revisions.py`
- `atr/shared/finish.py`
- `atr/get/compose.py`
- `atr/get/finish.py`
---
*Draft from a triage agent. A human reviewer should validate before merging any change. The agent did not run tests or verify diffs apply.*
--
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
Re: [I] Disallow phase transitions from any but the latest revision (tooling-trusted-releases)
Posted by "sbp (via GitHub)" <gi...@apache.org>.
sbp commented on issue #1182:
URL: https://github.com/apache/tooling-trusted-releases/issues/1182#issuecomment-4283561272
This only applies when going from compose to vote, because finish no longer has revisions.
--
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
Re: [I] Disallow phase transitions from any but the latest revision (tooling-trusted-releases)
Posted by "sbp (via GitHub)" <gi...@apache.org>.
sbp closed issue #1182: Disallow phase transitions from any but the latest revision
URL: https://github.com/apache/tooling-trusted-releases/issues/1182
--
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
Re: [I] Disallow phase transitions from any but the latest revision (tooling-trusted-releases)
Posted by "sbp (via GitHub)" <gi...@apache.org>.
sbp commented on issue #1182:
URL: https://github.com/apache/tooling-trusted-releases/issues/1182#issuecomment-4443543591
Resolved by c58bf39fda83ad458bf7d291f3b320663d6ed6aa.
--
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