PF-173: Add logs as artifact for a job#229
Conversation
WalkthroughBoth workflow files now sanitize working-directory fragments, upload Terragrunt log files as GitHub Actions artifacts when logs exist, and pass the resulting artifact URL into the update-comment step. ChangesTerragrunt log artifact wiring
Sequence Diagram(s)sequenceDiagram
participant pipelines_execute_job
participant terragrunt
participant upload_logs
participant update_comment
pipelines_execute_job->>terragrunt: sets pipelines_logs_artifact_path
pipelines_execute_job->>upload_logs: uploads logs when the path is non-empty
upload_logs-->>pipelines_execute_job: artifact-url
pipelines_execute_job->>update_comment: passes logs_artifact_url
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/pipelines.yml (1)
283-292: 🩺 Stability & Availability | 🔵 TrivialDownload Logs link remains safe.
upload-artifact@v6.0.0keeps theartifact-urloutput we need (plusartifact-idandartifact-digest), so the downstream step picking up the link is still on solid ground. The upgrade was mostly a runtime refresh, not an API break.Just remember to keep that safety net for empty logs: if
if-no-files-found: warnskips the upload, the URL will be empty, so the downstream step needs to handle a blank value gracefully.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pipelines.yml around lines 283 - 292, The upload-artifact step in pipelines.yml already exposes the artifact-url output, but the downstream logic that consumes the logs link must explicitly handle the case where no files were uploaded and the URL is empty. Update the step or follow-up consumer that reads upload_logs/artifact-url to treat a blank value as valid and skip or degrade gracefully instead of assuming a link always exists, using the existing Upload unit logs as artifact step as the reference point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/pipelines.yml:
- Around line 283-292: The upload-artifact step in pipelines.yml already exposes
the artifact-url output, but the downstream logic that consumes the logs link
must explicitly handle the case where no files were uploaded and the URL is
empty. Update the step or follow-up consumer that reads upload_logs/artifact-url
to treat a blank value as valid and skip or degrade gracefully instead of
assuming a link always exists, using the existing Upload unit logs as artifact
step as the reference point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9980119c-4861-4e02-a48f-7eb95c23cefb
📒 Files selected for processing (2)
.github/workflows/pipelines-root.yml.github/workflows/pipelines.yml
Implements plan for Linear issue PF-173.
Plan source: https://github.com/gruntwork-io-team/linear-claude-plans/blob/main/plans/PF-173.md
Claude's implementation summary
Related PRs in this change set
Summary by CodeRabbit
New Features
Bug Fixes