Fix basecall.config path and publish .nextflow.log to LOG_BUCKET

[katherine-stansifer]

In actually testing out basecalling lambda end-to-end I realized a few pieces of the entrypoint I'd left out--passing the config (absolutely essential for anything to run) and saving the .nextflow.log on S3 (not literally essential, but not having it makes debugging really hard)

Summary

• Point the automation entrypoint at the correct basecall.config so Nextflow picks up the intended params (c83803c).
• Wrap 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 (0d3af8c). Adds a required --log-bucket CLI arg and extends the docker-build smoke test to assert it.

Caveat — depends on Lambda + IAM changes

The --log-bucket flag is required, so this PR is not functional end-to-end until the concurrent changes in det-terraform-production-role ship:

• startOntBasecall Lambda must append --log-bucket <bucket> to containerOverrides.command.
• The Batch job role needs s3:PutObject on arn:aws:s3:::<log_bucket>/basecall-workflow/*.

I'm testing them both (from branches) in tandem and will only merge this when the whole system is working end-to-end.

Test plan

• CI Docker Build passes (the smoke test now asserts --log-bucket is listed in the argparse error output).
• After Lambda wiring lands: trigger lambda with a test_event.json (mocking an eventbridge from 1supplemental/barcodes.tsv), confirm s3://<log_bucket>/basecall-workflow/automated///.nextflow.log` exists once the Batch job terminates.

🤖 Generated with Claude Code

[katherine-stansifer]

review-pr

This review was generated by an automated code review tool (review-pr), not by the GitHub user whose account posted it. Findings reflect automated analysis and may not represent the views of the account owner.

Correctness

• C1 (suggestion): upload_nextflow_log only catches ClientError, but s3_client.upload_file can also raise boto3.exceptions.S3UploadFailedError (the transfer wrapper's preferred exception for non-AWS upload errors) and botocore.exceptions.BotoCoreError. Because the call lives in a finally block, an uncaught upload exception will replace any in-flight CalledProcessError from Nextflow and surface that instead of the underlying basecalling failure — the opposite of the docstring's stated intent ("log the error and return normally rather than raising"). Consider broadening to except Exception as e so the upload genuinely never masks the original failure, which is the only reason the function exists in the first place.

Testing

No issues found. The docker-build smoke test was extended to assert --log-bucket appears in argparse's required-args error output, matching the existing per-flag verification pattern. The repository has no Python test suite, so unit-level coverage for upload_nextflow_log isn't feasible here, and the PR description lays out the manual end-to-end test plan that gates merging.

Documentation

• D1 (issue): README.md (lines 80–84) lists the required CLI arguments for automation/run_automation.py — --delivery, --kit, --aws-queue, --base-bucket, --work-bucket — but does not include the newly-required --log-bucket. Anyone reading the README to wire up a caller, or comparing the Lambda's containerOverrides.command against the documented contract, will miss this. Add --log-bucket to that list with a one-line description matching the argparse help string.

Quality

No issues found. Hoisting s3_client above the try so it's shared between the upload and generate_samplesheet is a small, sensible refactor; the new helper is appropriately scoped; and the rest of the changes follow existing patterns in the file (typed signatures, log.info/log.warning levels, Path for filesystem locations).

Risks

No issues found. The PR description already calls out the cross-repo dependency (--log-bucket is required, so the matching Lambda + IAM changes in det-terraform-production-role must ship in tandem) and explicitly states this PR will not merge until the end-to-end path works.

[dp-rice]

One thing to check on re error handling, but looks good.