Fix #6182: Add Google Batch LogsPolicy PATH option for GCS bucket log…#6431
Fix #6182: Add Google Batch LogsPolicy PATH option for GCS bucket log…#6431bentsherman merged 21 commits intonextflow-io:masterfrom
Conversation
✅ Deploy Preview for nextflow-docs-staging canceled.
|
be1236d to
71f6f4a
Compare
… bucket log storage - Add logsBucket configuration option to BatchConfig - Support both CLOUD_LOGGING (default) and PATH log destinations - Add validation for GCS bucket paths (must start with gs://) - Include comprehensive tests for new functionality - Maintain backward compatibility 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: David Glazer <dglazer@verily.health>
71f6f4a to
1371aa6
Compare
- Add helper methods to extract bucket names and convert GCS paths to mount paths - Ensure logs bucket is mounted as Volume in GoogleBatchScriptLauncher - Update LogsPolicy to use container mount paths instead of GCS paths - Add comprehensive tests for bucket mounting and path conversion - Addresses reviewer feedback about missing bucket mounting requirements This ensures Google Batch can write logs to the specified GCS bucket by properly mounting it before referencing it in the LogsPolicy PATH. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Emma Rogge <emmarogge@verily.health>
1. Rename variable due to existing variable with same name. 2. Specify parameter type as string. Signed-off-by: Emma Rogge <emmarogge@verily.health>
1371aa6 to
b105b0e
Compare
|
@bentsherman I had a quick chat with @dglazer, this PR is ready for review (sounds like marking the PR as not draft can only be done by someone with write access? idk) |
|
@dglazer thanks for the contribution. What's the benefit of being able to send logs to a bucket instead of the standard cloud logging? |
|
Thanks for taking a look @bentsherman . The short answer is that it lets us provide a unified management experience to researchers, with the same ACLs controlling access to workflow inputs, outputs, and logs, and without them having to be familiar with GCP-native logging tools. @emmarogge can provide more details if needed. |
|
@dglazer @emmarogge I tried to push some updates to better incorporate your changes but for some reason it is not showing up on this PR. Can you see my commit from yesterday? |
|
I don't -- the most recent entry on the Commits tabs is from me merging master yesterday (3cdcdb0) |
2a1fd38 to
7df9723
Compare
|
Okay I think it's fixed now. You might want to do another round of testing to make sure everything still works on your end with my changes |
|
Thank you @bentsherman -- yes, we'll confirm things still work after your changes; I'll defer to @emmarogge on how quickly but it won't be long. |
|
I will do another round of testing now and report back -- thank you so much for the review @bentsherman ! |
|
Update: I have once again built Nextflow from this PR locally and tested out running some workflows with logging to a target GCS bucket configured. Logging appeared as expected in the bucket and the workflows succeeded. @bentsherman It's ready for your stamp when you have a chance! |
|
Thanks again for the contribution. This feature will show up in the next edge release (25.11.0-edge) which should be in the next few weeks |
… storage
🤖 Generated with Claude Code
NOTE: this change still needs human review.
Signed-off-by: David Glazer dglazer@verily.com