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 3b9ec6c5 Use quarantining for several revision creation methods
3b9ec6c5 is described below
commit 3b9ec6c5161950650b9eb784aa12b9fe6495d525
Author: Sean B. Palmer <[email protected]>
AuthorDate: Wed Mar 4 14:07:38 2026 +0000
Use quarantining for several revision creation methods
---
atr/api/__init__.py | 10 ++++++++--
atr/paths.py | 4 ++++
atr/post/upload.py | 20 ++++++++++++++++++--
atr/ssh.py | 22 ++++++++++++----------
atr/storage/writers/release.py | 18 +++++++++++-------
atr/tasks/quarantine.py | 21 +++++++++++++++------
atr/tasks/svn.py | 7 +++++--
playwright/test.py | 21 +++++++++++++--------
tests/e2e/announce/conftest.py | 8 +-------
tests/e2e/helpers.py | 14 ++++++++++++++
tests/e2e/vote/conftest.py | 8 +-------
tests/unit/test_quarantine_task.py | 21 ++++++++++++++++++---
12 files changed, 120 insertions(+), 54 deletions(-)
diff --git a/atr/api/__init__.py b/atr/api/__init__.py
index cf2d9ebf..ad2c628b 100644
--- a/atr/api/__init__.py
+++ b/atr/api/__init__.py
@@ -1003,10 +1003,16 @@ async def release_upload(data:
models.api.ReleaseUploadArgs) -> DictResponse:
async with storage.write(asf_uid) as write:
wacp = await write.as_project_committee_participant(data.project)
- revision = await wacp.release.upload_file(data)
+ result = await wacp.release.upload_file(data)
+ if isinstance(result, sql.Quarantined):
+ return {
+ "endpoint": "/release/upload",
+ "quarantined": True,
+ "message": "Upload received. Archive validation in progress.",
+ }, 202
return models.api.ReleaseUploadResults(
endpoint="/release/upload",
- revision=revision,
+ revision=result,
).model_dump(), 201
diff --git a/atr/paths.py b/atr/paths.py
index 4912d422..2c089e26 100644
--- a/atr/paths.py
+++ b/atr/paths.py
@@ -29,6 +29,10 @@ def get_attestable_dir() -> pathlib.Path:
return pathlib.Path(config.get().ATTESTABLE_STORAGE_DIR)
+def get_cache_archives_dir() -> pathlib.Path:
+ return pathlib.Path(config.get().STATE_DIR) / "cache" / "archives"
+
+
def get_downloads_dir() -> pathlib.Path:
return pathlib.Path(config.get().DOWNLOADS_STORAGE_DIR)
diff --git a/atr/post/upload.py b/atr/post/upload.py
index 305f077f..1a88c833 100644
--- a/atr/post/upload.py
+++ b/atr/post/upload.py
@@ -84,12 +84,20 @@ async def finalise(
dst = path / filename
await aioshutil.move(str(src), str(dst))
- await wacp.revision.create_revision(
+ result = await wacp.revision.create_revision_with_quarantine(
str(project_name), str(version_name), session.uid,
description=description, modify=modify
)
await aioshutil.rmtree(staging_dir)
+ if isinstance(result, sql.Quarantined):
+ return await session.redirect(
+ get.compose.selected,
+ success="Upload received. Archive validation in progress.",
+ project_name=str(project_name),
+ version_name=str(version_name),
+ )
+
return await session.redirect(
get.compose.selected,
success=f"{util.plural(number_of_files, 'file')} added
successfully",
@@ -187,7 +195,7 @@ async def _add_files(
async with storage.write(session) as write:
wacp = await write.as_project_committee_participant(project_name)
- creation_error, number_of_files = await wacp.release.upload_files(
+ creation_error, number_of_files, was_quarantined = await
wacp.release.upload_files(
project_name, version_name, file_name, file_data
)
@@ -199,6 +207,14 @@ async def _add_files(
version_name=version_name,
)
+ if was_quarantined:
+ return await session.redirect(
+ get.compose.selected,
+ success="Upload received. Archive validation in progress.",
+ project_name=project_name,
+ version_name=version_name,
+ )
+
return await session.redirect(
get.compose.selected,
success=f"{util.plural(number_of_files, 'file')} added
successfully",
diff --git a/atr/ssh.py b/atr/ssh.py
index b7aa4be3..25714bce 100644
--- a/atr/ssh.py
+++ b/atr/ssh.py
@@ -615,18 +615,20 @@ async def _step_07b_process_validated_rsync_write(
raise types.FailedError(f"rsync upload failed with exit status
{exit_status} for {for_revision}")
try:
- new_revision = await wacp.revision.create_revision(
+ result = await wacp.revision.create_revision_with_quarantine(
project_name, version_name, asf_uid, description=description,
modify=modify
)
- github_payload = server._get_github_payload(process)
- if github_payload is not None:
- await attestable.github_tp_payload_write(
- project_name, version_name, new_revision.number,
github_payload
- )
- log.info(f"rsync upload successful for revision
{new_revision.number}")
- host = config.get().APP_HOST
- message = f"\nATR: Created revision {new_revision.number} of
{project_name} {version_name}\n"
- message += f"ATR:
https://{host}/compose/{project_name}/{version_name}\n";
+ if isinstance(result, sql.Quarantined):
+ log.info(f"rsync upload quarantined for release
{release_name}")
+ message = f"\nATR: Upload received for {project_name}
{version_name}. Archive validation in progress.\n"
+ else:
+ github_payload = server._get_github_payload(process)
+ if github_payload is not None:
+ await attestable.github_tp_payload_write(project_name,
version_name, result.number, github_payload)
+ log.info(f"rsync upload successful for revision
{result.number}")
+ host = config.get().APP_HOST
+ message = f"\nATR: Created revision {result.number} of
{project_name} {version_name}\n"
+ message += f"ATR:
https://{host}/compose/{project_name}/{version_name}\n";
if not process.stderr.is_closing():
process.stderr.write(message.encode())
await process.stderr.drain()
diff --git a/atr/storage/writers/release.py b/atr/storage/writers/release.py
index 271d86c5..79735012 100644
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -107,6 +107,8 @@ class CommitteeParticipant(FoundationCommitter):
release_dirs = [
paths.release_directory_base(release),
paths.get_attestable_dir() / project_name / version,
+ paths.get_cache_archives_dir() / project_name / version,
+ paths.get_quarantined_dir() / project_name / version,
]
# Delete from the database using bulk SQL DELETE for efficiency
@@ -451,7 +453,7 @@ class CommitteeParticipant(FoundationCommitter):
)
return release, project
- async def upload_file(self, args: api.ReleaseUploadArgs) -> sql.Revision:
+ async def upload_file(self, args: api.ReleaseUploadArgs) -> sql.Revision |
sql.Quarantined:
file_bytes = base64.b64decode(args.content, validate=True)
validated_path = form.to_relpath(args.relpath)
if validated_path is None:
@@ -466,14 +468,16 @@ class CommitteeParticipant(FoundationCommitter):
async with aiofiles.open(target_path, "wb") as f:
await f.write(file_bytes)
- revision = await self.__write_as.revision.create_revision(
+ result = await
self.__write_as.revision.create_revision_with_quarantine(
args.project, args.version, self.__asf_uid,
description=description, modify=modify
)
+ if isinstance(result, sql.Quarantined):
+ return result
async with db.session() as data:
release_name = sql.release_name(args.project, args.version)
return await data.revision(
release_name=release_name,
- number=revision.number,
+ number=result.number,
).demand(storage.AccessError("Revision not found"))
async def upload_files(
@@ -482,7 +486,7 @@ class CommitteeParticipant(FoundationCommitter):
version_name: str,
file_name: pathlib.Path | None,
files: Sequence[datastructures.FileStorage],
- ) -> tuple[str | None, int]:
+ ) -> tuple[str | None, int, bool]:
"""Process and save the uploaded files into a new draft revision."""
number_of_files = len(files)
description = f"Upload of {util.plural(number_of_files, 'file')}
through web interface"
@@ -510,12 +514,12 @@ class CommitteeParticipant(FoundationCommitter):
await self.__save_file(file, target_path)
try:
- await self.__write_as.revision.create_revision(
+ result = await
self.__write_as.revision.create_revision_with_quarantine(
project_name, version_name, self.__asf_uid,
description=description, modify=modify
)
except types.FailedError as e:
- return str(e), len(files)
- return None, len(files)
+ return str(e), len(files), False
+ return None, len(files), isinstance(result, sql.Quarantined)
async def __current_paths(self, interim_path: pathlib.Path) ->
list[pathlib.Path]:
all_current_paths_interim: list[pathlib.Path] = []
diff --git a/atr/tasks/quarantine.py b/atr/tasks/quarantine.py
index 9ca22bbc..083780b5 100644
--- a/atr/tasks/quarantine.py
+++ b/atr/tasks/quarantine.py
@@ -79,22 +79,27 @@ async def validate(args: QuarantineValidate) ->
results.Results | None:
return None
try:
- await _extract_archives_to_cache(args.archives, quarantine_dir)
+ await _extract_archives_to_cache(args.archives, quarantine_dir,
project_name, version_name)
except Exception:
await _mark_failed(quarantined, file_entries, "Archive extraction to
cache failed")
await aioshutil.rmtree(quarantine_dir)
return None
- await _promote(quarantined, project_name, version_name, release,
str(quarantine_dir))
+ await _promote(quarantined, project_name, version_name, release.name,
str(quarantine_dir))
return None
-async def _extract_archives_to_cache(archives: list[QuarantineArchiveEntry],
quarantine_dir: pathlib.Path) -> None:
+async def _extract_archives_to_cache(
+ archives: list[QuarantineArchiveEntry],
+ quarantine_dir: pathlib.Path,
+ project_name: str,
+ version_name: str,
+) -> None:
# Cannot import as archives because that shadows the parameter name
import atr.archives
conf = config.get()
- cache_base = pathlib.Path(conf.STATE_DIR) / "cache" / "archives"
+ cache_base = paths.get_cache_archives_dir() / project_name / version_name
await aiofiles.os.makedirs(cache_base, exist_ok=True)
for archive in archives:
@@ -140,11 +145,15 @@ async def _promote(
quarantined: sql.Quarantined,
project_name: str,
version_name: str,
- release: sql.Release,
+ release_name: str,
quarantine_dir: str,
) -> None:
quarantine_dir_path = pathlib.Path(quarantine_dir)
- release_name = release.name
+
+ async with db.session() as data:
+ release = await data.release(name=release_name, _release_policy=True,
_project_release_policy=True).demand(
+ RuntimeError(f"Release {release_name} not found during quarantine
promotion")
+ )
path_to_hash, path_to_size = await
attestable.paths_to_hashes_and_sizes(quarantine_dir_path)
diff --git a/atr/tasks/svn.py b/atr/tasks/svn.py
index 3679fcc0..5b6c1829 100644
--- a/atr/tasks/svn.py
+++ b/atr/tasks/svn.py
@@ -122,10 +122,13 @@ async def _import_files_core(args: SvnImport) -> str:
await aiofiles.os.rmdir(temp_export_path)
log.info(f"Removed temporary export directory: {temp_export_path}")
- new_revision = await wacp.revision.create_revision(
+ result = await wacp.revision.create_revision_with_quarantine(
args.project_name, args.version_name, args.asf_uid,
description=description, modify=modify
)
- return f"Successfully imported files from SVN into revision
{new_revision.number}"
+ if isinstance(result, sql.Quarantined):
+ log.info(f"SVN import quarantined for
{args.project_name}-{args.version_name}")
+ return f"SVN import received for
{args.project_name}-{args.version_name}. Archive validation in progress."
+ return f"Successfully imported files from SVN into revision
{result.number}"
async def _import_files_core_run_svn_export(svn_command: list[str],
temp_export_path: pathlib.Path) -> None:
diff --git a/playwright/test.py b/playwright/test.py
index a82a37ae..d7e4db10 100755
--- a/playwright/test.py
+++ b/playwright/test.py
@@ -1154,17 +1154,22 @@ def test_ssh_02_rsync_upload(page: Page, credentials:
Credentials) -> None:
logging.error("rsync command not found. Is rsync installed in the
container?")
raise RuntimeError("rsync command not found")
- logging.info(f"Navigating to compose page for
{project_name}-{version_name}")
compose_path = f"/compose/{project_name}/{version_name}"
- go_to_path(page, compose_path)
- logging.info(f"Checking for uploaded files on {compose_path}")
-
- # Check for the existence of the files in the table using exact match
- file1_locator = page.get_by_role("cell", name=file1, exact=True)
- file2_locator = page.get_by_role("cell", name=file2, exact=True)
+ logging.info(f"Waiting for quarantine validation and files to appear on
{compose_path}")
+ max_wait_seconds = 30
+ start_time = time.monotonic()
+ while True:
+ go_to_path(page, compose_path)
+ file1_locator = page.get_by_role("cell", name=file1, exact=True)
+ if file1_locator.count() > 0:
+ break
+ elapsed = time.monotonic() - start_time
+ if elapsed > max_wait_seconds:
+ raise TimeoutError(f"Files did not appear on {compose_path} within
{max_wait_seconds} seconds")
+ time.sleep(1)
- expect(file1_locator).to_be_visible()
logging.info(f"Found file: {file1}")
+ file2_locator = page.get_by_role("cell", name=file2, exact=True)
expect(file2_locator).to_be_visible()
logging.info(f"Found file: {file2}")
logging.info("rsync upload test completed successfully")
diff --git a/tests/e2e/announce/conftest.py b/tests/e2e/announce/conftest.py
index 71a14105..b8c8ff82 100644
--- a/tests/e2e/announce/conftest.py
+++ b/tests/e2e/announce/conftest.py
@@ -69,8 +69,7 @@ def announce_context(browser: Browser) ->
Generator[BrowserContext]:
page.get_by_role("button", name="Add files").click()
page.wait_for_url(f"**/compose/{PROJECT_NAME}/{VERSION_NAME}")
- helpers.visit(page, f"/compose/{PROJECT_NAME}/{VERSION_NAME}")
- _wait_for_tasks_banner_hidden(page, timeout=60000)
+ helpers.wait_for_upload_and_tasks(page,
f"/compose/{PROJECT_NAME}/{VERSION_NAME}", FILE_NAME)
page.locator('a[title="Start a vote on this draft"]').click()
page.wait_for_load_state()
@@ -122,8 +121,3 @@ def _poll_for_vote_thread_link(page: Page, max_attempts:
int = 30) -> None:
return
time.sleep(0.5)
page.reload()
-
-
-def _wait_for_tasks_banner_hidden(page: Page, timeout: int = 30000) -> None:
- """Wait for all background tasks to be completed."""
- page.wait_for_selector("#ongoing-tasks-banner", state="hidden",
timeout=timeout)
diff --git a/tests/e2e/helpers.py b/tests/e2e/helpers.py
index bf86a145..76a0a979 100644
--- a/tests/e2e/helpers.py
+++ b/tests/e2e/helpers.py
@@ -16,6 +16,7 @@
# under the License.
import os
+import time
from typing import Any, Final
from playwright.sync_api import APIRequestContext, Page
@@ -48,3 +49,16 @@ def log_in(page: Page) -> None:
def visit(page: Page, path: str) -> None:
page.goto(f"{_ATR_BASE_URL}{path}")
page.wait_for_load_state()
+
+
+def wait_for_upload_and_tasks(page: Page, compose_url: str, file_name: str,
timeout: float = 60) -> None:
+ deadline = time.monotonic() + timeout
+ while True:
+ visit(page, compose_url)
+ if page.get_by_role("cell", name=file_name, exact=True).count() > 0:
+ break
+ if time.monotonic() > deadline:
+ raise TimeoutError(f"{file_name} did not appear on {compose_url}
within {timeout}s")
+ time.sleep(1)
+ remaining_ms = max(int((deadline - time.monotonic()) * 1000), 1000)
+ page.wait_for_selector("#ongoing-tasks-banner", state="hidden",
timeout=remaining_ms)
diff --git a/tests/e2e/vote/conftest.py b/tests/e2e/vote/conftest.py
index b689ed69..c2d0bec5 100644
--- a/tests/e2e/vote/conftest.py
+++ b/tests/e2e/vote/conftest.py
@@ -73,8 +73,7 @@ def vote_context(browser: Browser) ->
Generator[BrowserContext]:
page.get_by_role("button", name="Add files").click()
page.wait_for_url(f"**/compose/{PROJECT_NAME}/{VERSION_NAME}")
- helpers.visit(page, f"/compose/{PROJECT_NAME}/{VERSION_NAME}")
- _wait_for_tasks_banner_hidden(page, timeout=60000)
+ helpers.wait_for_upload_and_tasks(page,
f"/compose/{PROJECT_NAME}/{VERSION_NAME}", FILE_NAME)
page.locator('a[title="Start a vote on this draft"]').click()
page.wait_for_load_state()
@@ -87,8 +86,3 @@ def vote_context(browser: Browser) ->
Generator[BrowserContext]:
yield context
context.close()
-
-
-def _wait_for_tasks_banner_hidden(page: Page, timeout: int = 30000) -> None:
- """Wait for all background tasks to be completed."""
- page.wait_for_selector("#ongoing-tasks-banner", state="hidden",
timeout=timeout)
diff --git a/tests/unit/test_quarantine_task.py
b/tests/unit/test_quarantine_task.py
index 6f24c0f3..27119414 100644
--- a/tests/unit/test_quarantine_task.py
+++ b/tests/unit/test_quarantine_task.py
@@ -73,12 +73,24 @@ async def
test_promote_finalises_revision_and_deletes_quarantined(tmp_path: path
mock_safe_ctx.__aenter__ = mock.AsyncMock(return_value=mock.AsyncMock())
mock_safe_ctx.__aexit__ = mock.AsyncMock(return_value=False)
+ mock_release_query = mock.MagicMock()
+ mock_release_query.demand = mock.AsyncMock(return_value=release)
+
+ mock_release_data = mock.AsyncMock()
+ mock_release_data.release = mock.MagicMock(return_value=mock_release_query)
+
+ mock_release_ctx = mock.AsyncMock()
+ mock_release_ctx.__aenter__ =
mock.AsyncMock(return_value=mock_release_data)
+ mock_release_ctx.__aexit__ = mock.AsyncMock(return_value=False)
+
mock_delete_data = mock.AsyncMock()
mock_delete_data.delete = mock.AsyncMock()
mock_delete_ctx = mock.AsyncMock()
mock_delete_ctx.__aenter__ = mock.AsyncMock(return_value=mock_delete_data)
mock_delete_ctx.__aexit__ = mock.AsyncMock(return_value=False)
+ session_calls = iter([mock_release_ctx, mock_delete_ctx])
+
with (
mock.patch.object(
quarantine.attestable,
@@ -89,10 +101,13 @@ async def
test_promote_finalises_revision_and_deletes_quarantined(tmp_path: path
mock.patch.object(quarantine.util, "paths_to_inodes",
return_value={"file.txt": 12345}),
mock.patch.object(quarantine.revision, "SafeSession",
return_value=mock_safe_ctx),
mock.patch.object(quarantine.revision, "finalise_revision",
new_callable=mock.AsyncMock) as mock_finalise,
- mock.patch.object(quarantine.db, "session",
return_value=mock_delete_ctx),
+ mock.patch.object(quarantine.db, "session", side_effect=session_calls),
):
- await quarantine._promote(quarantined_row, "proj", "1.0", release,
quarantine_dir)
+ await quarantine._promote(quarantined_row, "proj", "1.0", "proj-1.0",
quarantine_dir)
+ mock_release_data.release.assert_called_once_with(
+ name="proj-1.0", _release_policy=True, _project_release_policy=True
+ )
mock_finalise.assert_awaited_once()
call_kwargs = mock_finalise.call_args.kwargs
assert call_kwargs["was_quarantined"] is True
@@ -268,7 +283,7 @@ async def test_validate_success_calls_promote(tmp_path:
pathlib.Path):
)
assert result is None
- mock_promote.assert_awaited_once_with(row, "proj", "1.0", row.release,
str(quarantine_dir))
+ mock_promote.assert_awaited_once_with(row, "proj", "1.0",
row.release.name, str(quarantine_dir))
mock_mark.assert_not_awaited()
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
