Improve Depthai Python CI/CD workflow - #1894
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe workflow computes ref-dependent wheel matrices in ChangesWheel workflow configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant precheck
participant WheelBuildJobs
participant CombinedWheelSmokeTests
PullRequest->>precheck: Provide event, ref, and labels
precheck->>WheelBuildJobs: Publish Python and runner matrices
WheelBuildJobs->>CombinedWheelSmokeTests: Build and combine wheels
CombinedWheelSmokeTests->>CombinedWheelSmokeTests: Select Python 3.14 interpreters
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
1ff1116 to
d0bd6d5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/workflows/python-main.yml:
- Line 21: Update the pull request event configuration in the workflow trigger
to include synchronize alongside the required pull request lifecycle events,
ensuring new commits on already-testable PRs start CI. Preserve the existing
precheck logic so it decides whether a run should be skipped.
- Around line 63-74: Gate every root work-producing wheel job on the `precheck`
job’s `should_run` output by adding the condition
`needs.precheck.outputs.should_run == 'true'` to each job’s `if`. Ensure the
existing `testable` label decision from the precheck step is enforced before any
downstream wheel work runs.
- Around line 411-414: Update the workflow steps using actions/checkout and
actions/setup-python to reference immutable, specific commit SHAs instead of the
mutable v3 and v4 major tags, preserving their existing behavior and
configuration.
- Around line 456-459: Update the container image in the workflow’s manylinux
build configuration to reference the same manylinux_2_28_x86_64 image by its
immutable `@sha256` digest instead of the mutable 2025.11.10-2 tag, while
preserving the existing PLAT environment value.
- Around line 43-45: Update the workflow step’s shell setup to pass github.ref
through the step-level env configuration, then read the environment variable
inside the run block instead of interpolating the GitHub expression into Bash.
Apply the same safe pattern to the existing GITHUB_REF assignment while
preserving the current variable names and behavior.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 09ed2042-2073-4441-939d-9ccb73a717ea
📒 Files selected for processing (1)
.github/workflows/python-main.yml
📜 Review details
🧰 Additional context used
🪛 actionlint (1.7.12)
.github/workflows/python-main.yml
[error] 411-411: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
[error] 412-412: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.26.1)
.github/workflows/python-main.yml
[warning] 44-44: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 45-45: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 411-411: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 411-411: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 412-412: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 457-457: unpinned image references (unpinned-images): container image is not pinned to a SHA256 hash
(unpinned-images)
aljazdu
changed the title
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/python-main.yml (1)
69-70: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire an exact
testablelabel match.
grep -q "testable"also matches labels such asnot-testableortestable-preview, so non-testable PRs can still launch the full wheel matrix. Use an exact match.Proposed fix
- if ! echo "$LABELS" | jq -r '.[].name' | grep -q "testable"; then + if ! echo "$LABELS" | jq -r '.[].name' | grep -Fxq "testable"; then🤖 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/python-main.yml around lines 69 - 70, Update the label check in the workflow’s SHOULD_RUN decision so it matches only a label whose complete name is exactly “testable”; use an exact-line match rather than substring matching, while preserving the existing false assignment for PRs without that label.
🤖 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.
Outside diff comments:
In @.github/workflows/python-main.yml:
- Around line 69-70: Update the label check in the workflow’s SHOULD_RUN
decision so it matches only a label whose complete name is exactly “testable”;
use an exact-line match rather than substring matching, while preserving the
existing false assignment for PRs without that label.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd3411e6-fed4-48f3-a970-5c192a790f97
📒 Files selected for processing (1)
.github/workflows/python-main.yml
📜 Review details
🔇 Additional comments (1)
.github/workflows/python-main.yml (1)
21-21: LGTM!Also applies to: 37-48, 58-68, 74-77, 240-244, 331-335, 412-412, 460-468, 567-567, 586-595, 697-697, 727-727
b705f17 to
c0ddf24
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/python-main.yml (1)
69-71: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire an exact
testablelabel match.
grep -q "testable"also matches labels such asnot-testableortestable-preview, allowing workflows to run without the required exact label.Proposed fix
- if ! echo "$LABELS" | jq -r '.[].name' | grep -q "testable"; then + if ! echo "$LABELS" | jq -r '.[].name' | grep -Fxq "testable"; then🤖 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/python-main.yml around lines 69 - 71, Update the label check in the workflow’s SHOULD_RUN decision to match the complete label value exactly as “testable,” preventing partial matches such as “not-testable” or “testable-preview.”
🤖 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/workflows/python-main.yml:
- Around line 62-64: Update the branch/tag condition controlling
WHEEL_MACOS_RUNNERS so refs/heads/develop is excluded, while retaining Intel
macOS runners for refs/heads/main, release branches, and v* tags.
---
Outside diff comments:
In @.github/workflows/python-main.yml:
- Around line 69-71: Update the label check in the workflow’s SHOULD_RUN
decision to match the complete label value exactly as “testable,” preventing
partial matches such as “not-testable” or “testable-preview.”
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb3876f2-b030-4346-97fb-8f4e964299f2
📒 Files selected for processing (1)
.github/workflows/python-main.yml
📜 Review details
🔇 Additional comments (1)
.github/workflows/python-main.yml (1)
21-21: LGTM!Also applies to: 33-48, 57-60, 66-68, 74-77, 239-243, 329-333, 407-413, 417-417, 465-465, 564-564, 582-582, 591-591, 693-693, 723-723
aljazkonec1
left a comment
aljazkonec1
left a comment
There was a problem hiding this comment.
Thanks! Lets just let workflow pass to see how it looks
aljazdu
added
testable
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/python-main.yml (1)
69-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMatch the
testablelabel exactly.
grep -q "testable"also matches labels such asnot-testableortestable-v2, causing wheel CI to run without the exact required label. Use an exact-match check such asgrep -Fxq "testable".🤖 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/python-main.yml around lines 69 - 71, Update the testable-label check in the SHOULD_RUN workflow condition to use exact line matching, such as grep -Fxq, so only the label exactly equal to “testable” qualifies; preserve the existing behavior for all other labels.
🤖 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.
Outside diff comments:
In @.github/workflows/python-main.yml:
- Around line 69-71: Update the testable-label check in the SHOULD_RUN workflow
condition to use exact line matching, such as grep -Fxq, so only the label
exactly equal to “testable” qualifies; preserve the existing behavior for all
other labels.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5e9a409e-f8e0-4774-bf64-bf5d8e46df12
📒 Files selected for processing (1)
.github/workflows/python-main.yml
📜 Review details
🔇 Additional comments (5)
.github/workflows/python-main.yml (5)
21-21: 🎯 Functional CorrectnessKeep
synchronizein the pull-request trigger.The supplied line is
types: [labeled]; unlesssynchronizeis declared elsewhere in this trigger, commits pushed to an already-testable PR will not rerun CI. This repeats the prior review finding—ensure the list remains[labeled, synchronize].
62-64: 🚀 Performance & ScalabilityExclude
developfrom legacy Intel macOS builds.This condition still enables
macos-15-intelfor everyrefs/heads/developrun, contrary to the stated objective that Intel builds run only formain, release branches, andv*tags. This repeats the prior review finding.
239-243: 🎯 Functional CorrectnessGate root wheel jobs on
should_run.
precheckpublishesshould_run=false, but these root wheel jobs have no job-level condition consuming it. A pull request without the exacttestablelabel can therefore still build wheels. Addif: needs.precheck.outputs.should_run == 'true'to the Windows, macOS, Linux x86_64, and Linux ARM64 build jobs; downstream combination jobs will then be skipped through their dependencies. This repeats the prior review finding.Proposed fix
build-windows-x86_64: needs: [precheck, build-docstrings] + if: needs.precheck.outputs.should_run == 'true' build-macos: needs: [precheck, build-docstrings] + if: needs.precheck.outputs.should_run == 'true' build-linux-x86_64: needs: [precheck, build-docstrings] + if: needs.precheck.outputs.should_run == 'true' build-linux-arm64: needs: [precheck, build-docstrings] + if: needs.precheck.outputs.should_run == 'true'Also applies to: 331-335, 459-467, 584-593
37-48: LGTM!Also applies to: 58-60, 66-68, 74-77
291-292: LGTM!Also applies to: 409-419, 566-566, 695-695, 725-725
dc54202 to
66ee70a
Compare
aljazdu
added
testable
66ee70a to
3355002
Compare
aljazdu
added
testable
3355002 to
d6d9f3d
Compare
aljazdu
added
testable
2 participants
Purpose
Specification
Updating the
python-main.ymlworkflow file with additional conditions.Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
This was tested by manually running the affected workflow with different conditions.
AI Usage
Assisted-by: AGENT_NAME: Chat gpt
Submitted code was reviewed by a human: YES
The author is taking the responsibility for the contribution: YES
Summary by CodeRabbit
Summary by CodeRabbit
Chores
Tests
cp310tocp314, and Windows from3.12to3.14).