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-release.git
The following commit(s) were added to refs/heads/main by this push:
new 15f847d Add a form to move release preview files, and improve path
construction
15f847d is described below
commit 15f847d0b8ad7f4ff7cb45a6201380414e0fca84
Author: Sean B. Palmer <[email protected]>
AuthorDate: Wed Apr 23 20:24:51 2025 +0100
Add a form to move release preview files, and improve path construction
---
atr/blueprints/admin/admin.py | 2 +-
atr/routes/candidate.py | 29 +++----
atr/routes/draft.py | 29 +++----
atr/routes/preview.py | 126 +++++++++++++++++++++++++++---
atr/routes/release.py | 2 +-
atr/tasks/__init__.py | 1 +
atr/templates/preview-refine-release.html | 47 +++++++++--
atr/util.py | 7 ++
playwright/test.py | 15 +++-
9 files changed, 200 insertions(+), 58 deletions(-)
diff --git a/atr/blueprints/admin/admin.py b/atr/blueprints/admin/admin.py
index 8773e2b..3347056 100644
--- a/atr/blueprints/admin/admin.py
+++ b/atr/blueprints/admin/admin.py
@@ -395,7 +395,7 @@ async def _delete_release_data(release_name: str) -> None:
release = await data.release(name=release_name).demand(
base.ASFQuartException(f"Release '{release_name}' not found.", 404)
)
- release_dir = util.release_directory(release)
+ release_dir = util.release_directory_base(release)
# Delete from the database
_LOGGER.info("Deleting database records for release: %s", release_name)
diff --git a/atr/routes/candidate.py b/atr/routes/candidate.py
index 4667575..e5168c8 100644
--- a/atr/routes/candidate.py
+++ b/atr/routes/candidate.py
@@ -147,14 +147,10 @@ async def view_path(
) -> response.Response | str:
"""View the content of a specific file in the release candidate."""
await session.check_access(project_name)
-
- async with db.session() as data:
- release = await data.release(name=models.release_name(project_name,
version_name), _project=True).demand(
- base.ASFQuartException("Release does not exist", errorcode=404)
- )
+ release = await session.release(project_name, version_name)
_max_view_size = 1 * 1024 * 1024
- full_path = util.get_release_candidate_dir() / project_name / version_name
/ file_path
+ full_path = util.release_directory(release) / file_path
content_listing = await util.archive_listing(full_path)
content, is_text, is_truncated, error_message = await
util.read_file_for_viewer(full_path, _max_view_size)
return await quart.render_template(
@@ -245,7 +241,7 @@ async def _resolve_post(session: routes.CommitterSession)
-> response.Response:
# Extract project name
try:
- project_name = candidate_name.rsplit("-", 1)[0]
+ project_name, version_name = candidate_name.rsplit("-", 1)
except ValueError:
return await session.redirect(resolve, error="Invalid candidate name
format")
@@ -255,13 +251,13 @@ async def _resolve_post(session: routes.CommitterSession)
-> response.Response:
# Update release status in the database
async with db.session() as data:
async with data.begin():
- release = await data.release(name=candidate_name,
_project=True).demand(
- routes.FlashError("Release candidate not found")
+ release = await session.release(
+ project_name, version_name,
phase=models.ReleasePhase.RELEASE_CANDIDATE, data=data
)
- # Verify that it's in the correct phase
- if release.phase != models.ReleasePhase.RELEASE_CANDIDATE:
- return await session.redirect(resolve, error="This release is
not in the voting phase")
+ # Get the source directory for the release candidate
+ # We need to do it here because we're updating the release status
in the database
+ source = str(util.release_directory(release))
# Update the release phase based on vote result
if vote_result == "passed":
@@ -272,15 +268,16 @@ async def _resolve_post(session: routes.CommitterSession)
-> response.Response:
release.phase = models.ReleasePhase.RELEASE_CANDIDATE_DRAFT
success_message = "Vote marked as failed"
- await _resolve_post_files(project_name, release, vote_result, session.uid)
+ await _resolve_post_files(project_name, release, source, vote_result,
session.uid)
return await session.redirect(
- preview.announce_release, success=success_message,
project_name=project_name, version_name=release.version
+ preview.refine_release, success=success_message,
project_name=project_name, version_name=release.version
)
-async def _resolve_post_files(project_name: str, release: models.Release,
vote_result: str, asf_uid: str) -> None:
+async def _resolve_post_files(
+ project_name: str, release: models.Release, source: str, vote_result: str,
asf_uid: str
+) -> None:
# TODO: Obtain a lock for this
- source = str(util.get_release_candidate_dir() / project_name /
release.version)
if vote_result != "passed":
# The vote failed, so move the release candidate to the release draft
directory
async with revision.create_and_manage(project_name, release.version,
asf_uid) as (
diff --git a/atr/routes/draft.py b/atr/routes/draft.py
index d27da8c..49fb7db 100644
--- a/atr/routes/draft.py
+++ b/atr/routes/draft.py
@@ -250,7 +250,7 @@ async def compose(session: routes.CommitterSession,
project_name: str, version_n
await session.check_access(project_name)
release = await session.release(project_name, version_name)
- base_path = util.get_release_candidate_draft_dir() / project_name /
version_name / release.unwrap_revision
+ base_path = util.release_directory(release)
paths = await util.paths_recursive(base_path)
path_templates = {}
path_substitutions = {}
@@ -414,6 +414,7 @@ async def delete(session: routes.CommitterSession) ->
response.Response:
return await session.redirect(drafts, error=f"Error deleting
candidate draft: {e!s}")
# Delete the files on disk, including all revisions
+ # We can't use util.release_directory_base here because we don't have the
release object
draft_dir = util.get_release_candidate_draft_dir() / project_name / version
if await aiofiles.os.path.exists(draft_dir):
# Believe this to be another bug in mypy Protocol handling
@@ -512,7 +513,7 @@ async def evaluate(session: routes.CommitterSession,
project_name: str, version_
await session.check_access(project_name)
release = await session.release(project_name, version_name,
with_committee=True, with_project=False)
- base_path = util.get_release_candidate_draft_dir() / project_name /
version_name / release.unwrap_revision
+ base_path = util.release_directory(release)
paths = await util.paths_recursive(base_path)
# paths_set = set(paths)
path_templates = {}
@@ -550,9 +551,7 @@ async def evaluate(session: routes.CommitterSession,
project_name: str, version_
path_metadata.add(path)
# Get modified time
- full_path = str(
- util.get_release_candidate_draft_dir() / project_name /
version_name / release.unwrap_revision / path
- )
+ full_path = str(util.release_directory(release) / path)
path_modified[path] = int(await aiofiles.os.path.getmtime(full_path))
# Get successes, warnings, and errors
@@ -607,7 +606,7 @@ async def evaluate_path(session: routes.CommitterSession,
project_name: str, ver
release = await session.release(project_name, version_name)
# TODO: When we do more than one thing in a dir, we should use the
revision directory directly
- abs_path = util.get_release_candidate_draft_dir() / project_name /
version_name / release.unwrap_revision / rel_path
+ abs_path = util.release_directory(release) / rel_path
# Check that the file exists
if not await aiofiles.os.path.exists(abs_path):
@@ -726,12 +725,11 @@ async def revision_set(session: routes.CommitterSession,
project_name: str, vers
async with db.session() as data:
try:
release = await session.release(project_name, version_name,
data=data)
- release_dir = util.get_release_candidate_draft_dir() /
project_name / version_name
except base.ASFQuartException:
release = await session.release(
project_name, version_name,
phase=models.ReleasePhase.RELEASE_PREVIEW, data=data
)
- release_dir = util.get_release_preview_dir() / project_name /
version_name
+ release_dir = util.release_directory_base(release)
# Check that the target revision directory exists
target_revision_dir = release_dir / revision_name
@@ -767,12 +765,11 @@ async def revisions(session: routes.CommitterSession,
project_name: str, version
try:
release = await session.release(project_name, version_name)
- release_dir = util.get_release_candidate_draft_dir() / project_name /
version_name
phase_key = "draft"
except base.ASFQuartException:
release = await session.release(project_name, version_name,
phase=models.ReleasePhase.RELEASE_PREVIEW)
- release_dir = util.get_release_preview_dir() / project_name /
version_name
phase_key = "preview"
+ release_dir = util.release_directory_base(release)
revision_dirs: list[str] = []
with contextlib.suppress(FileNotFoundError):
@@ -1009,9 +1006,7 @@ async def tools(session: routes.CommitterSession,
project_name: str, version_nam
await session.check_access(project_name)
release = await session.release(project_name, version_name)
- full_path = str(
- util.get_release_candidate_draft_dir() / project_name / version_name /
release.unwrap_revision / file_path
- )
+ full_path = str(util.release_directory(release) / file_path)
# Check that the file exists
if not await aiofiles.os.path.exists(full_path):
@@ -1074,9 +1069,7 @@ async def view_path(
# Limit to 256 KiB
_max_view_size = 256 * 1024
- full_path = (
- util.get_release_candidate_draft_dir() / project_name / version_name /
release.unwrap_revision / file_path
- )
+ full_path = util.release_directory(release) / file_path
# Attempt to get an archive listing
# This will be None if the file is not an archive
@@ -1327,7 +1320,7 @@ async def _promote(
if release.phase != models.ReleasePhase.RELEASE_CANDIDATE_DRAFT:
return "This release is not in the candidate draft phase"
- base_dir = util.get_release_candidate_draft_dir() / project_name /
version_name
+ base_dir = util.release_directory_base(release)
# Use the directory of the specified revision
# TODO: This ensures that we promote the correct revision, but does not
stop conflicts
# We need to obtain a lock when promoting
@@ -1345,7 +1338,7 @@ async def _promote(
release.stage = models.ReleaseStage.RELEASE_CANDIDATE
release.phase = models.ReleasePhase.RELEASE_CANDIDATE
release.revision = None
- target_dir = util.get_release_candidate_dir() / project_name / version_name
+ target_dir = util.release_directory(release)
if await aiofiles.os.path.exists(target_dir):
return f"Target directory {target_dir.name} already exists"
diff --git a/atr/routes/preview.py b/atr/routes/preview.py
index 8e428c9..f970e13 100644
--- a/atr/routes/preview.py
+++ b/atr/routes/preview.py
@@ -18,6 +18,8 @@
"""preview.py"""
import logging
+import pathlib
+from typing import Final
import aiofiles.os
import aioshutil
@@ -28,6 +30,7 @@ import wtforms
import atr.db as db
import atr.db.models as models
+import atr.revision as revision
import atr.routes as routes
import atr.routes.release as routes_release
import atr.util as util
@@ -35,6 +38,26 @@ import atr.util as util
if asfquart.APP is ...:
raise RuntimeError("APP is not set")
+_LOGGER: Final = logging.getLogger(__name__)
+
+
+class MoveFileForm(util.QuartFormTyped):
+ """Form for moving a file within a preview revision."""
+
+ source_file = wtforms.SelectField("File to move", choices=[],
validators=[wtforms.validators.InputRequired()])
+ target_directory = wtforms.SelectField(
+ "Target directory", choices=[],
validators=[wtforms.validators.InputRequired()]
+ )
+ submit = wtforms.SubmitField("Move file")
+
+ def validate_target_directory(self, field: wtforms.Field) -> None:
+ # This validation runs only if both fields have data
+ if self.source_file.data and field.data:
+ source_path = pathlib.Path(self.source_file.data)
+ target_dir = pathlib.Path(field.data)
+ if source_path.parent == target_dir:
+ raise wtforms.validators.ValidationError("Target directory
cannot be the same as the source directory.")
+
class AnnounceForm(util.QuartFormTyped):
"""Form for announcing a release preview."""
@@ -90,17 +113,13 @@ async def announce(session: routes.CommitterSession) ->
str | response.Response:
if (quart.request.method == "POST") and (await
announce_form.validate_on_submit()):
try:
- preview_name, project_name, version_name, preview_revision =
_announce_form_validate(announce_form)
+ _preview_name, project_name, version_name, _preview_revision =
_announce_form_validate(announce_form)
except ValueError as e:
return await session.redirect(announce, error=str(e))
# Check that the user has access to the project
async with db.session() as data:
- project = await data.project(name=project_name).get()
- if not project:
- return await session.redirect(announce, error="Project not
found")
- if not any((p.id == project.id) for p in (await
session.user_projects)):
- return await session.redirect(announce, error="You do not have
access to this project")
+ await session.check_access(project_name)
try:
# Get the release
@@ -112,15 +131,18 @@ async def announce(session: routes.CommitterSession) ->
str | response.Response:
# Impossible, but to satisfy the type checkers
return await session.redirect(announce, error="This
release does not have a revision")
- source_base = util.get_release_preview_dir() / project_name /
version_name
+ source_base = util.release_directory_base(release)
source = str(source_base / release.revision)
- target = str(util.get_release_dir() / project_name /
version_name)
- if await aiofiles.os.path.exists(target):
- return await session.redirect(announce, error="Release
already exists")
# Update the database
release.phase = models.ReleasePhase.RELEASE
release.revision = None
+
+ # This must come after updating the release object
+ target = str(util.release_directory(release))
+ if await aiofiles.os.path.exists(target):
+ return await session.redirect(announce, error="Release
already exists")
+
await data.commit()
# Move the revision directory
@@ -195,6 +217,7 @@ async def delete(session: routes.CommitterSession) ->
response.Response:
return await session.redirect(announce, error=f"Error deleting
preview: {e!s}")
# Delete the files on disk, including all revisions
+ # We can't use util.release_directory_base here because we don't have the
release object
preview_dir = util.get_release_preview_dir() / project_name / version
if await aiofiles.os.path.exists(preview_dir):
await aioshutil.rmtree(preview_dir)
@@ -221,14 +244,43 @@ async def previews(session: routes.CommitterSession) ->
str:
)
[email protected]("/refine/<project_name>/<version_name>")
[email protected]("/refine/<project_name>/<version_name>", methods=["GET",
"POST"])
async def refine_release(
session: routes.CommitterSession, project_name: str, version_name: str
) -> response.Response | str:
"""Refine a release preview."""
await session.check_access(project_name)
release = await session.release(project_name, version_name,
phase=models.ReleasePhase.RELEASE_PREVIEW)
- return await quart.render_template("preview-refine-release.html",
release=release)
+
+ current_revision_dir = util.release_directory(release)
+ file_paths_rel: list[pathlib.Path] = []
+ unique_dirs: set[pathlib.Path] = {pathlib.Path(".")}
+
+ try:
+ for path in await util.paths_recursive(current_revision_dir):
+ file_paths_rel.append(path)
+ unique_dirs.add(path.parent)
+ except FileNotFoundError:
+ await quart.flash("Preview revision directory not found.", "error")
+ return await session.redirect(previews)
+
+ form = await MoveFileForm.create_form(data=await quart.request.form if
(quart.request.method == "POST") else None)
+
+ # Populate choices dynamically for both GET and POST
+ form.source_file.choices = sorted([(str(p), str(p)) for p in
file_paths_rel])
+ form.target_directory.choices = sorted([(str(d), str(d)) for d in
unique_dirs])
+ can_move = len(unique_dirs) > 1
+
+ if (quart.request.method == "POST") and can_move:
+ match r := await _move_file(form, session, project_name, version_name):
+ case None:
+ pass
+ case response.Response():
+ return r
+
+ return await quart.render_template(
+ "preview-refine-release.html", release=release,
file_paths=sorted(file_paths_rel), form=form, can_move=can_move
+ )
@routes.committer("/preview/view/<project_name>/<version_name>")
@@ -267,7 +319,7 @@ async def view_path(
release = await session.release(project_name, version_name,
phase=models.ReleasePhase.RELEASE_PREVIEW)
_max_view_size = 1 * 1024 * 1024
- full_path = util.get_release_preview_dir() / project_name / version_name /
release.unwrap_revision / file_path
+ full_path = util.release_directory(release) / file_path
content_listing = await util.archive_listing(full_path)
content, is_text, is_truncated, error_message = await
util.read_file_for_viewer(full_path, _max_view_size)
return await quart.render_template(
@@ -325,3 +377,51 @@ async def _delete_preview(data: db.Session, preview_name:
str) -> None:
# Delete the release record
await data.delete(release)
+
+
+async def _move_file(
+ form: MoveFileForm, session: routes.CommitterSession, project_name: str,
version_name: str
+) -> response.Response | None:
+ if await form.validate_on_submit():
+ source_file_rel = pathlib.Path(form.source_file.data)
+ target_dir_rel = pathlib.Path(form.target_directory.data)
+
+ try:
+ async with revision.create_and_manage(project_name, version_name,
session.uid, preview=True) as (
+ new_revision_dir,
+ new_revision_name,
+ ):
+ source_path_in_new = new_revision_dir / source_file_rel
+ target_path_in_new = new_revision_dir / target_dir_rel /
source_file_rel.name
+
+ if await aiofiles.os.path.exists(target_path_in_new):
+ await quart.flash(
+ f"File '{source_file_rel.name}' already exists in
'{target_dir_rel}' in new revision.",
+ "error",
+ )
+ return await session.redirect(refine_release,
project_name=project_name, version_name=version_name)
+
+ _LOGGER.info(f"Moving {source_path_in_new} to
{target_path_in_new} in new revision {new_revision_name}")
+ await aiofiles.os.rename(source_path_in_new,
target_path_in_new)
+
+ await quart.flash(
+ f"File '{source_file_rel.name}' moved successfully to
'{target_dir_rel}' in new revision.", "success"
+ )
+ return await session.redirect(refine_release,
project_name=project_name, version_name=version_name)
+
+ except FileNotFoundError:
+ _LOGGER.exception("File not found during move operation in new
revision")
+ await quart.flash("Error: Source file not found during move
operation.", "error")
+ except OSError as e:
+ _LOGGER.exception("Error moving file in new revision")
+ await quart.flash(f"Error moving file: {e}", "error")
+ except Exception as e:
+ _LOGGER.exception("Unexpected error during file move")
+ await quart.flash(f"An unexpected error occurred: {e!s}", "error")
+ return await session.redirect(refine_release,
project_name=project_name, version_name=version_name)
+ else:
+ for field, errors in form.errors.items():
+ field_label = getattr(getattr(form, field, None), "label", None)
+ label_text = field_label.text if field_label else
field.replace("_", " ").title()
+ for error in errors:
+ await quart.flash(f"{label_text}: {error}", "warning")
diff --git a/atr/routes/release.py b/atr/routes/release.py
index 20d29ac..c022a6f 100644
--- a/atr/routes/release.py
+++ b/atr/routes/release.py
@@ -148,7 +148,7 @@ async def view_path(
"""View the content of a specific file in the final release."""
release = await session.release(project_name, version_name)
_max_view_size = 1 * 1024 * 1024
- full_path = util.get_release_dir() / project_name / version_name /
file_path
+ full_path = util.release_directory(release) / file_path
content_listing = await util.archive_listing(full_path)
content, is_text, is_truncated, error_message = await
util.read_file_for_viewer(full_path, _max_view_size)
return await quart.render_template(
diff --git a/atr/tasks/__init__.py b/atr/tasks/__init__.py
index 61e86d0..436e0e7 100644
--- a/atr/tasks/__init__.py
+++ b/atr/tasks/__init__.py
@@ -59,6 +59,7 @@ async def draft_checks(
) -> int:
"""Core logic to analyse a draft revision and queue checks."""
# Construct path to the specific revision
+ # We don't have the release object here, so we can't use
util.release_directory
revision_path = util.get_release_candidate_draft_dir() / project_name /
release_version / draft_revision
relative_paths = await util.paths_recursive(revision_path)
diff --git a/atr/templates/preview-refine-release.html
b/atr/templates/preview-refine-release.html
index 7c337fa..8f97c02 100644
--- a/atr/templates/preview-refine-release.html
+++ b/atr/templates/preview-refine-release.html
@@ -57,15 +57,48 @@
</div>
</div>
- <div class="card mb-4 shadow-sm">
- <div class="card-header bg-light">
- <h3 class="card-title mb-0">TODO</h3>
+ <div class="alert alert-warning mb-4" role="alert">
+ <p class="fw-semibold mb-1">TODO</p>
+ <p class="mb-1">
+ Planned enhancements include removing "RC" tags from filenames (not yet
implemented) and improving the file movement interface, as the current
dropdowns are unsuitable for large releases.
+ </p>
+ </div>
+
+ {% if can_move %}
+ <div class="card mb-4">
+ <div class="card-header bg-light">
+ <h3 class="mb-0">Move file to different directory</h3>
+ </div>
+ <div class="card-body">
+ <form method="post" class="atr-canary">
+ {{ form.hidden_tag() }}
+ <div class="mb-3">
+ {{ form.source_file.label(class="form-label") }}
+ {{ form.source_file(class="form-select form-select-sm
font-monospace") }}
+ {% if form.source_file.errors %}
+ <div class="invalid-feedback d-block">
+ {% for error in form.source_file.errors %}{{ error }}{% endfor
%}
+ </div>
+ {% endif %}
+ </div>
+ <div class="mb-3">
+ {{ form.target_directory.label(class="form-label") }}
+ {{ form.target_directory(class="form-select form-select-sm
font-monospace") }}
+ {% if form.target_directory.errors %}
+ <div class="invalid-feedback d-block">
+ {% for error in form.target_directory.errors %}{{ error }}{%
endfor %}
+ </div>
+ {% endif %}
+ </div>
+ {{ form.submit(class="btn btn-primary btn-sm") }}
+ </form>
+ </div>
</div>
- <div class="card-body">
- <p>We plan to allow moving files and removing "RC" tags from files on
this page.</p>
- <p>For now, you can only move straight to the announce page.</p>
+ {% else %}
+ <div class="alert alert-info" role="alert">
+ File moving is disabled as all files are currently in the same directory
or the revision is empty.
</div>
- </div>
+ {% endif %}
{% endblock content %}
{% block javascripts %}
diff --git a/atr/util.py b/atr/util.py
index 7c3f276..0963120 100644
--- a/atr/util.py
+++ b/atr/util.py
@@ -395,6 +395,13 @@ async def read_file_for_viewer(full_path: pathlib.Path,
max_size: int) -> tuple[
def release_directory(release: models.Release) -> pathlib.Path:
+ """Return the absolute path to the directory containing the active files
for a given release phase."""
+ if release.revision is None:
+ return release_directory_base(release)
+ return release_directory_base(release) / release.revision
+
+
+def release_directory_base(release: models.Release) -> pathlib.Path:
"""Determine the filesystem directory for a given release based on its
phase."""
phase = release.phase
try:
diff --git a/playwright/test.py b/playwright/test.py
index b1ffafc..93b4f9d 100644
--- a/playwright/test.py
+++ b/playwright/test.py
@@ -206,12 +206,23 @@ def lifecycle_05_resolve_vote(page: sync_api.Page,
credentials: Credentials, ver
sync_api.expect(submit_button_locator).to_be_enabled()
submit_button_locator.click()
- logging.info(f"Waiting for navigation to
/announce/tooling-test-example/{version_name} after resolving the vote")
- wait_for_path(page, f"/announce/tooling-test-example/{version_name}")
+ logging.info(f"Waiting for navigation to
/refine/tooling-test-example/{version_name} after resolving the vote")
+ wait_for_path(page, f"/refine/tooling-test-example/{version_name}")
logging.info("Vote resolution actions completed successfully")
def lifecycle_06_announce_preview(page: sync_api.Page, credentials:
Credentials, version_name: str) -> None:
+ go_to_path(page, f"/refine/tooling-test-example/{version_name}")
+ logging.info("Refine page loaded successfully")
+
+ logging.info(f"Locating the announce link for tooling-test-example
{version_name}")
+ announce_link_locator =
page.locator(f'a[href="/announce/tooling-test-example/{esc_id(version_name)}"]')
+ sync_api.expect(announce_link_locator).to_be_visible()
+ announce_link_locator.click()
+
+ logging.info(f"Waiting for navigation to
/announce/tooling-test-example/{version_name} after announcing preview")
+ wait_for_path(page, f"/announce/tooling-test-example/{version_name}")
+
logging.info(f"Locating the announcement form for tooling-test-example
{version_name}")
form_locator = page.locator('form[action="/preview/announce"]')
sync_api.expect(form_locator).to_be_visible()
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
