From c83803c13e6f3e4aeedefbc3212e1a63b1117388 Mon Sep 17 00:00:00 2001 From: katherine-stansifer Date: Thu, 14 May 2026 21:45:06 -0400 Subject: [PATCH 1/6] add correct config file to automation --- automation/run_automation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/automation/run_automation.py b/automation/run_automation.py index b171c12..8a2fe9e 100644 --- a/automation/run_automation.py +++ b/automation/run_automation.py @@ -74,6 +74,7 @@ def build_nextflow_cmd( """ return [ "nextflow", "run", "/workflow/main.nf", + "-c", "/workflow/configs/basecall.config", "-profile", "batch", "--nanopore_run", delivery, "--kit", kit, From 0d3af8c526df1721905745f86ec616a90cfc4bca Mon Sep 17 00:00:00 2001 From: katherine-stansifer Date: Fri, 15 May 2026 13:21:23 -0400 Subject: [PATCH 2/6] Publish .nextflow.log to LOG_BUCKET after each automation run Wraps the Nextflow subprocess in a try/finally that uploads /workflow/.nextflow.log to s3://{log_bucket}/basecall-workflow/automated/ {delivery}/{ts}/ on both success and failure, so failed Fargate head jobs are debuggable post-mortem instead of losing the log on container teardown. Upload errors are logged and swallowed to avoid masking Nextflow's real exit code. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/docker-build.yml | 2 +- automation/run_automation.py | 34 ++++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index a01b83d..6cc8d63 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -52,6 +52,6 @@ jobs: echo "Expected argparse exit code 2 for missing required args; got $exit_code" exit 1 fi - for flag in --delivery --kit --aws-queue --base-bucket --work-bucket; do + for flag in --delivery --kit --aws-queue --base-bucket --work-bucket --log-bucket; do echo "$output" | grep -q -- "$flag" || { echo "Missing required flag in error output: $flag"; exit 1; } done diff --git a/automation/run_automation.py b/automation/run_automation.py index 8a2fe9e..7bbfa5f 100644 --- a/automation/run_automation.py +++ b/automation/run_automation.py @@ -18,11 +18,14 @@ """ import argparse +import datetime as dt import logging import subprocess +from pathlib import Path import boto3 from botocore.config import Config +from botocore.exceptions import ClientError from seq_import.samplesheet import generate_samplesheet log = logging.getLogger(__name__) @@ -54,6 +57,10 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: "--work-bucket", required=True, help="S3 bucket for Nextflow's working directory", ) + parser.add_argument( + "--log-bucket", required=True, + help="S3 bucket for publishing .nextflow.log after the run", + ) return parser.parse_args(argv) @@ -85,6 +92,26 @@ def build_nextflow_cmd( ] +def upload_nextflow_log( + s3_client, delivery: str, log_bucket: str, log_path: Path = Path("/workflow/.nextflow.log"), +) -> None: + """Upload .nextflow.log to s3://{log_bucket}/basecall-workflow/automated/{delivery}/{ts}/. + + Logged-and-swallowed on failure: a log upload problem should never mask + the real exit status of the Nextflow run. + """ + if not log_path.exists(): + log.warning("No %s to upload", log_path) + return + timestamp = dt.datetime.now(dt.UTC).strftime("%Y%m%d_%H%M%S") + s3_key = f"basecall-workflow/automated/{delivery}/{timestamp}/.nextflow.log" + try: + s3_client.upload_file(str(log_path), log_bucket, s3_key) + log.info("Uploaded nextflow log to s3://%s/%s", log_bucket, s3_key) + except ClientError as e: + log.warning("Failed to upload %s: %s", log_path, e) + + def main() -> None: logging.basicConfig( level=logging.INFO, @@ -100,10 +127,13 @@ def main() -> None: args.work_bucket, ) log.info("Running nextflow: %s", " ".join(nextflow_cmd)) - subprocess.run(nextflow_cmd, check=True, cwd="/workflow") + s3_client = boto3.client("s3", config=Config(max_pool_connections=50)) + try: + subprocess.run(nextflow_cmd, check=True, cwd="/workflow") + finally: + upload_nextflow_log(s3_client, args.delivery, args.log_bucket) log.info("Generating samplesheet for delivery %s in bucket %s", args.delivery, args.base_bucket) - s3_client = boto3.client("s3", config=Config(max_pool_connections=50)) output_path = generate_samplesheet(s3_client, args.delivery, bucket=args.base_bucket) log.info("Samplesheet written to: %s", output_path) From 4b2ee781f551860fc50ca496f7e7c644daa25182 Mon Sep 17 00:00:00 2001 From: katherine-stansifer Date: Fri, 15 May 2026 13:49:23 -0400 Subject: [PATCH 3/6] Log nextflow log upload failures as errors, not warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So a misconfigured IAM policy or bucket name shows up in CloudWatch error metrics instead of being buried in warnings — without raising, which would block the samplesheet generation step from running on an otherwise successful Nextflow run. Co-Authored-By: Claude Opus 4.7 (1M context) --- automation/run_automation.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/automation/run_automation.py b/automation/run_automation.py index 7bbfa5f..69a9295 100644 --- a/automation/run_automation.py +++ b/automation/run_automation.py @@ -97,8 +97,11 @@ def upload_nextflow_log( ) -> None: """Upload .nextflow.log to s3://{log_bucket}/basecall-workflow/automated/{delivery}/{ts}/. - Logged-and-swallowed on failure: a log upload problem should never mask - the real exit status of the Nextflow run. + If the upload fails, log the error and return normally rather than + raising. We call this from a finally block before generate_samplesheet, + so raising would block samplesheet generation on a successful Nextflow + run — worse than losing a log file. The error log surfaces the failure + in CloudWatch error metrics so a misconfigured IAM/bucket gets noticed. """ if not log_path.exists(): log.warning("No %s to upload", log_path) @@ -109,7 +112,7 @@ def upload_nextflow_log( s3_client.upload_file(str(log_path), log_bucket, s3_key) log.info("Uploaded nextflow log to s3://%s/%s", log_bucket, s3_key) except ClientError as e: - log.warning("Failed to upload %s: %s", log_path, e) + log.error("Failed to upload %s: %s", log_path, e) def main() -> None: From 8c284a41071a3e78a0659ef58ef4574af803bafc Mon Sep 17 00:00:00 2001 From: katherine-stansifer Date: Fri, 15 May 2026 14:27:56 -0400 Subject: [PATCH 4/6] Guard .nextflow.log upload from masking Nextflow failures; document --log-bucket Wrap the upload_nextflow_log call inside the finally block with an outer try/except Exception, so an unexpected upload error can't replace an in-flight CalledProcessError from Nextflow. Mirrors the two-layer pattern used in mgs-orchestrator (automation/run_automation.py:213-220): inner helper catches ClientError, outer call site catches Exception. Also add --log-bucket to the Automation section's required-arguments list in README.md so callers see the full Lambda containerOverrides.command contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 1 + automation/run_automation.py | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 0af3096..70cbb9e 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ Required arguments (passed by the `startOntBasecall` Lambda via Batch `container - `--aws-queue` — AWS Batch GPU queue for child basecalling jobs - `--base-bucket` — S3 bucket holding the delivery (`raw/`, `supplemental/`, `metadata/`) - `--work-bucket` — S3 bucket for Nextflow's working directory +- `--log-bucket` — S3 bucket for publishing `.nextflow.log` after the run ### Build-time authentication diff --git a/automation/run_automation.py b/automation/run_automation.py index 69a9295..73bdde2 100644 --- a/automation/run_automation.py +++ b/automation/run_automation.py @@ -134,7 +134,13 @@ def main() -> None: try: subprocess.run(nextflow_cmd, check=True, cwd="/workflow") finally: - upload_nextflow_log(s3_client, args.delivery, args.log_bucket) + # Outer guard so an unexpected upload exception can't replace an + # in-flight CalledProcessError from Nextflow. Mirrors the pattern in + # mgs-orchestrator's automation/run_automation.py. + try: + upload_nextflow_log(s3_client, args.delivery, args.log_bucket) + except Exception as e: + log.exception("Failed to upload .nextflow.log: %s", e) log.info("Generating samplesheet for delivery %s in bucket %s", args.delivery, args.base_bucket) output_path = generate_samplesheet(s3_client, args.delivery, bucket=args.base_bucket) From 834920db3183d614298cf773ad3a8bdba3bc4591 Mon Sep 17 00:00:00 2001 From: katherine-stansifer Date: Fri, 15 May 2026 15:07:18 -0400 Subject: [PATCH 5/6] Simplify .nextflow.log upload guard to a single except Exception MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit mirrored mgs-orchestrator's two-layer exception pattern, but that pattern is justified there by a multi-file upload loop and a larger try-block scope. For our single-file, single-call upload, broadening the helper's catch to `except Exception` (with `log.exception` for the traceback) gives the same safety property — the finally block can't mask a Nextflow CalledProcessError — without the call-site nesting. Co-Authored-By: Claude Opus 4.7 (1M context) --- automation/run_automation.py | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/automation/run_automation.py b/automation/run_automation.py index 73bdde2..a6b1e71 100644 --- a/automation/run_automation.py +++ b/automation/run_automation.py @@ -25,7 +25,6 @@ import boto3 from botocore.config import Config -from botocore.exceptions import ClientError from seq_import.samplesheet import generate_samplesheet log = logging.getLogger(__name__) @@ -97,11 +96,11 @@ def upload_nextflow_log( ) -> None: """Upload .nextflow.log to s3://{log_bucket}/basecall-workflow/automated/{delivery}/{ts}/. - If the upload fails, log the error and return normally rather than - raising. We call this from a finally block before generate_samplesheet, - so raising would block samplesheet generation on a successful Nextflow - run — worse than losing a log file. The error log surfaces the failure - in CloudWatch error metrics so a misconfigured IAM/bucket gets noticed. + Called from a finally block, so we swallow all exceptions: raising on a + successful Nextflow run would block samplesheet generation, and raising + on a failed Nextflow run would replace the original CalledProcessError + with an upload error. The error log surfaces the failure in CloudWatch + so a misconfigured IAM/bucket gets noticed. """ if not log_path.exists(): log.warning("No %s to upload", log_path) @@ -111,8 +110,8 @@ def upload_nextflow_log( try: s3_client.upload_file(str(log_path), log_bucket, s3_key) log.info("Uploaded nextflow log to s3://%s/%s", log_bucket, s3_key) - except ClientError as e: - log.error("Failed to upload %s: %s", log_path, e) + except Exception as e: + log.exception("Failed to upload %s: %s", log_path, e) def main() -> None: @@ -134,13 +133,7 @@ def main() -> None: try: subprocess.run(nextflow_cmd, check=True, cwd="/workflow") finally: - # Outer guard so an unexpected upload exception can't replace an - # in-flight CalledProcessError from Nextflow. Mirrors the pattern in - # mgs-orchestrator's automation/run_automation.py. - try: - upload_nextflow_log(s3_client, args.delivery, args.log_bucket) - except Exception as e: - log.exception("Failed to upload .nextflow.log: %s", e) + upload_nextflow_log(s3_client, args.delivery, args.log_bucket) log.info("Generating samplesheet for delivery %s in bucket %s", args.delivery, args.base_bucket) output_path = generate_samplesheet(s3_client, args.delivery, bucket=args.base_bucket) From df39c656fdaabeead01ed4ef6283557d5c3809a6 Mon Sep 17 00:00:00 2001 From: katherine-stansifer Date: Thu, 21 May 2026 17:23:14 -0400 Subject: [PATCH 6/6] Move finally-block rationale from upload_nextflow_log docstring to its call site The docstring now states the function's contract (logs and swallows all upload errors); the finally call site explains why that contract is required there. Suggested by dp-rice in PR #27 review. --- automation/run_automation.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/automation/run_automation.py b/automation/run_automation.py index a6b1e71..c7f9988 100644 --- a/automation/run_automation.py +++ b/automation/run_automation.py @@ -96,11 +96,8 @@ def upload_nextflow_log( ) -> None: """Upload .nextflow.log to s3://{log_bucket}/basecall-workflow/automated/{delivery}/{ts}/. - Called from a finally block, so we swallow all exceptions: raising on a - successful Nextflow run would block samplesheet generation, and raising - on a failed Nextflow run would replace the original CalledProcessError - with an upload error. The error log surfaces the failure in CloudWatch - so a misconfigured IAM/bucket gets noticed. + Logs and swallows all upload errors so it is safe to call from a finally block. + The error log surfaces the failure in CloudWatch so a misconfigured IAM/bucket gets noticed. """ if not log_path.exists(): log.warning("No %s to upload", log_path) @@ -133,6 +130,10 @@ def main() -> None: try: subprocess.run(nextflow_cmd, check=True, cwd="/workflow") finally: + # upload_nextflow_log is designed to log-and-swallow all exceptions. + # (Because an exception in this finally block would prevent + # samplesheet generation on a successful Nextflow run, or mask CalledProcessError + # from a failed one.) upload_nextflow_log(s3_client, args.delivery, args.log_bucket) log.info("Generating samplesheet for delivery %s in bucket %s", args.delivery, args.base_bucket)