feat: Publish Toolkit artifacts as OCI images#106
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds OCI artifact publishing to the CD pipeline. A new composite action ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
📦 Python package built successfully!
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #106 +/- ##
=======================================
Coverage 74.09% 74.09%
=======================================
Files 95 95
Lines 5678 5678
Branches 843 843
=======================================
Hits 4207 4207
Misses 1195 1195
Partials 276 276
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In @.github/actions/push-oci-artifact/action.yml:
- Around line 53-56: The debug path is hardcoded to "dist/" which can mislead
callers; update the missing-file handling around the check for ${file_path} to
list the parent directory of the provided file instead of dist/. Use dirname on
${file_path} (e.g., compute parent_dir="$(dirname "${file_path}")") and run ls
-la on that parent_dir (with a fallback message if it doesn't exist) so logs
reflect the actual location passed in; ensure you keep the existing error exit
behavior.
In @.github/workflows/cd.yml:
- Around line 183-187: The hardcoded list under the "files:" key (e.g.,
dist/constraints3.10.txt, dist/constraints3.11.txt, etc.) will drift if the
workflow matrix changes; replace the static list with a dynamic generation tied
to the matrix (use matrix.python-version or a step that iterates over
matrix.python-version to build the list) or at minimum add a clear comment next
to "files:" mentioning the coupling to the matrix defined earlier; update
references to the per-version filenames (dist/constraints3.10.txt etc.) to be
produced from the matrix values so changes to matrix.python-version
automatically update the artifact list.
- Around line 206-213: The current conditional block using the variables
build_result and constraints_result doesn't handle the 'skipped' state and will
treat skipped jobs as failures; update the logic in the if/elif/else block that
starts with if [[ $build_result == "success" && $constraints_result == "success"
]] to explicitly handle 'skipped' (e.g., treat 'skipped' as non-fatal/success or
add an elif branch for skipped) so that when either build_result or
constraints_result is "skipped" you return a clear message and the desired exit
code (likely exit 0) instead of falling through to the failure else branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d7ac127-9952-4896-a261-6acb1dfbb0f2
📒 Files selected for processing (2)
.github/actions/push-oci-artifact/action.yml.github/workflows/cd.yml
|
🚀 Review App Deployment Started
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/actions/push-oci-artifact/action.yml:
- Around line 56-65: The current loop copies each file to
"${rootfs_dir}/$(basename "${file_path}")", which can silently overwrite files
with the same basename; fix it by preserving the source path under rootfs_dir
(or at minimum detecting collisions) before copying: compute
target_dir="$(dirname "${rootfs_dir}/${file_path}")", mkdir -p "$target_dir",
and cp "${file_path}" "${target_dir}/$(basename "${file_path}")"; alternatively,
if you prefer flat targets, check if the destination exists (using [ -e
"${rootfs_dir}/$(basename "${file_path}")" ]) and exit with an error that
includes both source and existing destination to avoid silent overwrites in the
loop that reads FILES and uses file_path/rootfs_dir.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a46045a1-4d95-46e5-ae7f-97ffcc758fde
📒 Files selected for processing (1)
.github/actions/push-oci-artifact/action.yml
…rtifacts-as-oci-images
Summary by CodeRabbit