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

commit c27221bc86dd1dd624a5b334818be7b307a37f49
Author: Sean B. Palmer <[email protected]>
AuthorDate: Fri May 2 10:05:34 2025 +0100

    Fix interface order
---
 atr/routes/preview.py |  68 ++++++++++++------------
 atr/routes/resolve.py |  78 ++++++++++++++--------------
 atr/ssh.py            | 140 +++++++++++++++++++++++++-------------------------
 3 files changed, 143 insertions(+), 143 deletions(-)

diff --git a/atr/routes/preview.py b/atr/routes/preview.py
index 541745a..495c5f4 100644
--- a/atr/routes/preview.py
+++ b/atr/routes/preview.py
@@ -37,6 +37,13 @@ if asfquart.APP is ...:
     raise RuntimeError("APP is not set")
 
 
+class AnnouncePreviewForm(util.QuartFormTyped):
+    """Form for validating preview request data."""
+
+    subject = wtforms.StringField("Subject", 
validators=[wtforms.validators.Optional()])
+    body = wtforms.TextAreaField("Body", 
validators=[wtforms.validators.InputRequired("Body is required for preview")])
+
+
 class DeleteForm(util.QuartFormTyped):
     """Form for deleting a release preview."""
 
@@ -53,11 +60,34 @@ class DeleteForm(util.QuartFormTyped):
     submit = wtforms.SubmitField("Delete preview")
 
 
-class AnnouncePreviewForm(util.QuartFormTyped):
-    """Form for validating preview request data."""
[email protected]("/preview/announce/<project_name>/<version_name>", 
methods=["POST"])
+async def announce_preview(
+    session: routes.CommitterSession, project_name: str, version_name: str
+) -> quart.wrappers.response.Response | str:
+    """Generate a preview of the announcement email body."""
 
-    subject = wtforms.StringField("Subject", 
validators=[wtforms.validators.Optional()])
-    body = wtforms.TextAreaField("Body", 
validators=[wtforms.validators.InputRequired("Body is required for preview")])
+    form = await AnnouncePreviewForm.create_form(data=await quart.request.form)
+    if not await form.validate_on_submit():
+        error_message = "Invalid preview request"
+        if form.errors:
+            error_details = "; ".join([f"{field}: {', '.join(errs)}" for 
field, errs in form.errors.items()])
+            error_message = f"{error_message}: {error_details}"
+        return quart.Response(f"Error: {error_message}", status=400, 
mimetype="text/plain")
+
+    try:
+        # Construct options and generate body
+        options = construct.AnnounceReleaseOptions(
+            asfuid=session.uid,
+            project_name=project_name,
+            version_name=version_name,
+        )
+        preview_body = await 
construct.announce_release_body(str(form.body.data), options)
+
+        return quart.Response(preview_body, mimetype="text/plain")
+
+    except Exception as e:
+        logging.exception("Error generating announcement preview:")
+        return quart.Response(f"Error generating preview: {e!s}", status=500, 
mimetype="text/plain")
 
 
 @routes.committer("/preview/delete", methods=["POST"])
@@ -136,36 +166,6 @@ async def view(session: routes.CommitterSession, 
project_name: str, version_name
     )
 
 
[email protected]("/preview/announce/<project_name>/<version_name>", 
methods=["POST"])
-async def announce_preview(
-    session: routes.CommitterSession, project_name: str, version_name: str
-) -> quart.wrappers.response.Response | str:
-    """Generate a preview of the announcement email body."""
-
-    form = await AnnouncePreviewForm.create_form(data=await quart.request.form)
-    if not await form.validate_on_submit():
-        error_message = "Invalid preview request"
-        if form.errors:
-            error_details = "; ".join([f"{field}: {', '.join(errs)}" for 
field, errs in form.errors.items()])
-            error_message = f"{error_message}: {error_details}"
-        return quart.Response(f"Error: {error_message}", status=400, 
mimetype="text/plain")
-
-    try:
-        # Construct options and generate body
-        options = construct.AnnounceReleaseOptions(
-            asfuid=session.uid,
-            project_name=project_name,
-            version_name=version_name,
-        )
-        preview_body = await 
construct.announce_release_body(str(form.body.data), options)
-
-        return quart.Response(preview_body, mimetype="text/plain")
-
-    except Exception as e:
-        logging.exception("Error generating announcement preview:")
-        return quart.Response(f"Error generating preview: {e!s}", status=500, 
mimetype="text/plain")
-
-
 
@routes.committer("/preview/view/<project_name>/<version_name>/<path:file_path>")
 async def view_path(
     session: routes.CommitterSession, project_name: str, version_name: str, 
file_path: str
diff --git a/atr/routes/resolve.py b/atr/routes/resolve.py
index d001a7c..748adcc 100644
--- a/atr/routes/resolve.py
+++ b/atr/routes/resolve.py
@@ -46,6 +46,22 @@ class ResolveForm(util.QuartFormTyped):
     submit = wtforms.SubmitField("Resolve vote")
 
 
+def release_latest_vote_task(release: models.Release) -> models.Task | None:
+    # Find the most recent VOTE_INITIATE task for this release
+    # TODO: Make this a proper query
+    for task in sorted(release.tasks, key=lambda t: t.added, reverse=True):
+        if task.task_type != models.TaskType.VOTE_INITIATE:
+            continue
+        # if task.status != models.TaskStatus.COMPLETED:
+        #     continue
+        if (task.status == models.TaskStatus.QUEUED) or (task.status == 
models.TaskStatus.ACTIVE):
+            continue
+        if task.result is None:
+            continue
+        return task
+    return None
+
+
 @routes.committer("/resolve/<project_name>/<version_name>", 
measure_performance=False)
 async def selected(session: routes.CommitterSession, project_name: str, 
version_name: str) -> response.Response | str:
     """Resolve the vote on a release candidate."""
@@ -140,6 +156,29 @@ async def selected_post(
     )
 
 
+def task_mid_get(latest_vote_task: models.Task) -> str | None:
+    # TODO: Improve this
+    task_mid = None
+
+    try:
+        for result in latest_vote_task.result or []:
+            if isinstance(result, str):
+                parsed_result = json.loads(result)
+            else:
+                # Shouldn't happen
+                parsed_result = result
+            if isinstance(parsed_result, dict):
+                task_mid = parsed_result.get("mid", "(mid not found in 
result)")
+                break
+            else:
+                task_mid = "(malformed result)"
+
+    except (json.JSONDecodeError, TypeError):
+        task_mid = "(malformed result)"
+
+    return task_mid
+
+
 def _format_artifact_name(project_name: str, version: str, is_podling: bool = 
False) -> str:
     """Format an artifact name according to Apache naming conventions.
 
@@ -153,22 +192,6 @@ def _format_artifact_name(project_name: str, version: str, 
is_podling: bool = Fa
     return f"apache-{project_name}-{version}"
 
 
-def release_latest_vote_task(release: models.Release) -> models.Task | None:
-    # Find the most recent VOTE_INITIATE task for this release
-    # TODO: Make this a proper query
-    for task in sorted(release.tasks, key=lambda t: t.added, reverse=True):
-        if task.task_type != models.TaskType.VOTE_INITIATE:
-            continue
-        # if task.status != models.TaskStatus.COMPLETED:
-        #     continue
-        if (task.status == models.TaskStatus.QUEUED) or (task.status == 
models.TaskStatus.ACTIVE):
-            continue
-        if task.result is None:
-            continue
-        return task
-    return None
-
-
 async def _task_archive_url(task_mid: str) -> str | None:
     if "@" not in task_mid:
         return None
@@ -215,26 +238,3 @@ async def _task_archive_url_cached(task_mid: str | None) 
-> str | None:
         )
 
     return url
-
-
-def task_mid_get(latest_vote_task: models.Task) -> str | None:
-    # TODO: Improve this
-    task_mid = None
-
-    try:
-        for result in latest_vote_task.result or []:
-            if isinstance(result, str):
-                parsed_result = json.loads(result)
-            else:
-                # Shouldn't happen
-                parsed_result = result
-            if isinstance(parsed_result, dict):
-                task_mid = parsed_result.get("mid", "(mid not found in 
result)")
-                break
-            else:
-                task_mid = "(malformed result)"
-
-    except (json.JSONDecodeError, TypeError):
-        task_mid = "(malformed result)"
-
-    return task_mid
diff --git a/atr/ssh.py b/atr/ssh.py
index 2c3df34..9b20785 100644
--- a/atr/ssh.py
+++ b/atr/ssh.py
@@ -115,7 +115,7 @@ async def server_start() -> asyncssh.SSHAcceptor:
     server = await asyncssh.create_server(
         SSHServer,
         server_host_keys=[key_path],
-        process_factory=_01_handle_client,
+        process_factory=_step_01_handle_client,
         host=_CONFIG.SSH_HOST,
         port=_CONFIG.SSH_PORT,
         encoding=None,
@@ -132,7 +132,21 @@ async def server_stop(server: asyncssh.SSHAcceptor) -> 
None:
     _LOGGER.info("SSH server stopped")
 
 
-async def _01_handle_client(process: asyncssh.SSHServerProcess) -> None:
+def _fail(process: asyncssh.SSHServerProcess, message: str, return_value: T) 
-> T:
+    _LOGGER.error(message)
+    # Ensure message is encoded before writing to stderr
+    encoded_message = f"ATR SSH error: {message}\n".encode()
+    try:
+        process.stderr.write(encoded_message)
+    except BrokenPipeError:
+        _LOGGER.warning("Failed to write error to client stderr: Broken pipe")
+    except Exception as e:
+        _LOGGER.exception(f"Error writing to client stderr: {e}")
+    process.exit(1)
+    return return_value
+
+
+async def _step_01_handle_client(process: asyncssh.SSHServerProcess) -> None:
     """Process client command, validating and dispatching to read or write 
handlers."""
     asf_uid = process.get_extra_info("username")
     _LOGGER.info(f"Handling command for authenticated user: {asf_uid}")
@@ -144,17 +158,17 @@ async def _01_handle_client(process: 
asyncssh.SSHServerProcess) -> None:
     # TODO: Use shlex.split or similar if commands can contain quoted arguments
     argv = process.command.split()
 
-    #########################################
-    ### Calls _02_command_simple_validate ###
-    #########################################
-    simple_validation_error, path_index, is_read_request = 
_02_command_simple_validate(argv)
+    ##############################################
+    ### Calls _step_02_command_simple_validate ###
+    ##############################################
+    simple_validation_error, path_index, is_read_request = 
_step_02_command_simple_validate(argv)
     if simple_validation_error:
         return _fail(process, f"{simple_validation_error}\nCommand: 
{process.command}", None)
 
-    ##################################
-    ### Calls _04_command_validate ###
-    ##################################
-    validation_results = await _04_command_validate(process, argv, path_index, 
is_read_request)
+    #######################################
+    ### Calls _step_04_command_validate ###
+    #######################################
+    validation_results = await _step_04_command_validate(process, argv, 
path_index, is_read_request)
     if not validation_results:
         return
 
@@ -167,19 +181,19 @@ async def _01_handle_client(process: 
asyncssh.SSHServerProcess) -> None:
             # This should not happen if the validation logic is correct
             return _fail(process, "Internal error: Release object missing for 
read request after validation", None)
         _LOGGER.info(f"Processing READ request for 
{project_name}-{version_name}")
-        ###############################################
-        ### Calls _07a_process_validated_rsync_read ###
-        ###############################################
-        await _07a_process_validated_rsync_read(process, argv, path_index, 
release_obj)
+        ####################################################
+        ### Calls _step_07a_process_validated_rsync_read ###
+        ####################################################
+        await _step_07a_process_validated_rsync_read(process, argv, 
path_index, release_obj)
     else:
         _LOGGER.info(f"Processing WRITE request for 
{project_name}-{version_name}")
-        ################################################
-        ### Calls _07b_process_validated_rsync_write ###
-        ################################################
-        await _07b_process_validated_rsync_write(process, argv, path_index, 
project_name, version_name)
+        #####################################################
+        ### Calls _step_07b_process_validated_rsync_write ###
+        #####################################################
+        await _step_07b_process_validated_rsync_write(process, argv, 
path_index, project_name, version_name)
 
 
-def _02_command_simple_validate(argv: list[str]) -> tuple[str | None, int, 
bool]:
+def _step_02_command_simple_validate(argv: list[str]) -> tuple[str | None, 
int, bool]:
     """Validate the basic structure of the rsync command and detect read vs 
write."""
     # READ: ['rsync', '--server', '--sender', '-vlogDtpre.iLsfxCIvu', '.', 
'/proj/v1/']
     # WRITE: ['rsync', '--server', '-vlogDtpre.iLsfxCIvu', '.', '/proj/v1/']
@@ -213,17 +227,17 @@ def _02_command_simple_validate(argv: list[str]) -> 
tuple[str | None, int, bool]
     if options.split("e.", 1)[0] != "-vlogDtpr":
         return "The options argument (after --sender) must be 
'-vlogDtpre.[compatibility flags]'", -1, True
 
-    ###############################################
-    ### Calls _03_validate_rsync_args_structure ###
-    ###############################################
-    error, path_index = _03_validate_rsync_args_structure(argv, option_index, 
is_read_request)
+    ####################################################
+    ### Calls _step_03_validate_rsync_args_structure ###
+    ####################################################
+    error, path_index = _step_03_validate_rsync_args_structure(argv, 
option_index, is_read_request)
     if error:
         return error, -1, is_read_request
 
     return None, path_index, is_read_request
 
 
-def _03_validate_rsync_args_structure(
+def _step_03_validate_rsync_args_structure(
     argv: list[str], option_index: int, is_read_request: bool
 ) -> tuple[str | None, int]:
     """Validate the dot argument and path argument presence and count."""
@@ -254,14 +268,14 @@ def _03_validate_rsync_args_structure(
     return None, path_index
 
 
-async def _04_command_validate(
+async def _step_04_command_validate(
     process: asyncssh.SSHServerProcess, argv: list[str], path_index: int, 
is_read_request: bool
 ) -> tuple[str, str, models.Release | None] | None:
     """Validate the path and user permissions for read or write."""
-    #######################################
-    ### Calls _05_command_path_validate ###
-    #######################################
-    result = _05_command_path_validate(argv[path_index])
+    ############################################
+    ### Calls _step_05_command_path_validate ###
+    ############################################
+    result = _step_05_command_path_validate(argv[path_index])
     if isinstance(result, str):
         return _fail(process, result, None)
     path_project, path_version = result
@@ -277,27 +291,27 @@ async def _04_command_validate(
         release = await data.release(project_name=project.name, 
version=path_version).get()
 
         if is_read_request:
-            ############################################
-            ### Calls _06a_validate_read_permissions ###
-            ############################################
-            validated_release, success = await _06a_validate_read_permissions(
+            #################################################
+            ### Calls _step_06a_validate_read_permissions ###
+            #################################################
+            validated_release, success = await 
_step_06a_validate_read_permissions(
                 process, ssh_uid, project, release, path_project, path_version
             )
             if success is None:
                 return None
             return path_project, path_version, validated_release
         else:
-            #############################################
-            ### Calls _06b_validate_write_permissions ###
-            #############################################
-            success = await _06b_validate_write_permissions(process, ssh_uid, 
project, release)
+            ##################################################
+            ### Calls _step_06b_validate_write_permissions ###
+            ##################################################
+            success = await _step_06b_validate_write_permissions(process, 
ssh_uid, project, release)
             if success is None:
                 return None
             # Return None for the release object for write requests
             return path_project, path_version, None
 
 
-def _05_command_path_validate(path: str) -> tuple[str, str] | str:
+def _step_05_command_path_validate(path: str) -> tuple[str, str] | str:
     """Validate the path argument for rsync commands."""
     # READ: rsync --server --sender -vlogDtpre.iLsfxCIvu . /proj/v1/
     # Validating path: /proj/v1/
@@ -338,7 +352,7 @@ def _05_command_path_validate(path: str) -> tuple[str, str] 
| str:
     return path_project, path_version
 
 
-async def _06a_validate_read_permissions(
+async def _step_06a_validate_read_permissions(
     process: asyncssh.SSHServerProcess,
     ssh_uid: str,
     project: models.Project,
@@ -370,7 +384,7 @@ async def _06a_validate_read_permissions(
     return release, True
 
 
-async def _06b_validate_write_permissions(
+async def _step_06b_validate_write_permissions(
     process: asyncssh.SSHServerProcess,
     ssh_uid: str,
     project: models.Project,
@@ -404,7 +418,7 @@ async def _06b_validate_write_permissions(
     return True
 
 
-async def _07a_process_validated_rsync_read(
+async def _step_07a_process_validated_rsync_read(
     process: asyncssh.SSHServerProcess,
     argv: list[str],
     path_index: int,
@@ -429,10 +443,10 @@ async def _07a_process_validated_rsync_read(
         if not argv[path_index].endswith("/"):
             argv[path_index] += "/"
 
-        ##############################################
-        ### Calls _08_execute_rsync_sender_command ###
-        ##############################################
-        exit_status = await _08_execute_rsync(process, argv)
+        ###################################################
+        ### Calls _step_08_execute_rsync_sender_command ###
+        ###################################################
+        exit_status = await _step_08_execute_rsync(process, argv)
         if exit_status != 0:
             _LOGGER.error(
                 f"rsync --sender failed with exit status {exit_status} for 
release {release.name}. "
@@ -447,7 +461,7 @@ async def _07a_process_validated_rsync_read(
         process.exit(1)
 
 
-async def _07b_process_validated_rsync_write(
+async def _step_07b_process_validated_rsync_write(
     process: asyncssh.SSHServerProcess,
     argv: list[str],
     path_index: int,
@@ -461,10 +475,10 @@ async def _07b_process_validated_rsync_write(
     try:
         # Ensure the release object exists or is created
         # This must happen before creating the revision directory
-        ###################################################
-        ### Calls _07b2_ensure_release_object_for_write ###
-        ###################################################
-        if not await _07b2_ensure_release_object_for_write(process, 
project_name, version_name):
+        #######################################################
+        ### Calls _step_07c_ensure_release_object_for_write ###
+        #######################################################
+        if not await _step_07c_ensure_release_object_for_write(process, 
project_name, version_name):
             # The _fail function was already called in 
_07b2_ensure_release_object_for_write
             return
 
@@ -477,10 +491,10 @@ async def _07b_process_validated_rsync_write(
             # Update the rsync command path to the new revision directory
             argv[path_index] = str(new_revision_dir)
 
-            ##############################################
-            ### Calls _08_execute_rsync_upload_command ###
-            ##############################################
-            exit_status = await _08_execute_rsync(process, argv)
+            ###################################################
+            ### Calls _step_08_execute_rsync_upload_command ###
+            ###################################################
+            exit_status = await _step_08_execute_rsync(process, argv)
             if exit_status != 0:
                 _LOGGER.error(
                     f"rsync upload failed with exit status {exit_status} for 
revision {new_draft_revision}. "
@@ -498,7 +512,7 @@ async def _07b_process_validated_rsync_write(
         process.exit(1)
 
 
-async def _07b2_ensure_release_object_for_write(
+async def _step_07c_ensure_release_object_for_write(
     process: asyncssh.SSHServerProcess, project_name: str, version_name: str
 ) -> bool:
     """Ensure the release object exists or create it for a write operation."""
@@ -539,7 +553,7 @@ async def _07b2_ensure_release_object_for_write(
         return _fail(process, f"Internal error ensuring release object: {e}", 
False)
 
 
-async def _08_execute_rsync(process: asyncssh.SSHServerProcess, argv: 
list[str]) -> int:
+async def _step_08_execute_rsync(process: asyncssh.SSHServerProcess, argv: 
list[str]) -> int:
     """Execute the modified rsync command."""
     _LOGGER.info(f"Executing modified rsync command: {' '.join(argv)}")
     proc = await asyncio.create_subprocess_shell(
@@ -554,17 +568,3 @@ async def _08_execute_rsync(process: 
asyncssh.SSHServerProcess, argv: list[str])
     exit_status = await proc.wait()
     _LOGGER.info(f"Rsync finished with exit status {exit_status}")
     return exit_status
-
-
-def _fail(process: asyncssh.SSHServerProcess, message: str, return_value: T) 
-> T:
-    _LOGGER.error(message)
-    # Ensure message is encoded before writing to stderr
-    encoded_message = f"ATR SSH error: {message}\n".encode()
-    try:
-        process.stderr.write(encoded_message)
-    except BrokenPipeError:
-        _LOGGER.warning("Failed to write error to client stderr: Broken pipe")
-    except Exception as e:
-        _LOGGER.exception(f"Error writing to client stderr: {e}")
-    process.exit(1)
-    return return_value


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to