ci(cross-platform): add LFX wheel artifact handling and tests #9703
Conversation
WalkthroughAdds optional LFX wheel artifact handling to cross-platform tests, passes LFX artifact name from release and nightly workflows, and introduces a dedicated cross-platform LFX test job in release. Updates job dependencies to include LFX build artifacts and conditionally installs LFX wheels when requested. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Workflow Caller
participant CPT as cross-platform-test (reusable)
participant DL as Download LFX Artifact
participant Install as Install Wheel via uv pip
participant Tests as Test Steps
Dev->>CPT: inputs.method, inputs.lfx-artifact-name
alt method == "wheel" AND lfx-artifact-name != ""
CPT->>DL: Download artifact to ./lfx-dist
DL-->>CPT: .whl found or error
CPT->>Install: Use OS-specific interpreter (uv pip install ./lfx-dist/*.whl)
Install-->>CPT: Installed or failure
else
note over CPT: Follow existing non-LFX path (PyPI/artifacts)
end
CPT->>Tests: Run stable/experimental test steps
Tests-->>CPT: Results
sequenceDiagram
autonumber
participant R as release.yml
participant BBase as build-base
participant BMain as build-main
participant BLFX as build-lfx
participant TCPT as test-cross-platform
participant TLFX as test-lfx-cross-platform
R->>BBase: run
R->>BMain: run
R->>BLFX: run (when release_lfx == true)
par After builds complete
BBase-->>TCPT: needs
BMain-->>TCPT: needs
BLFX-->>TCPT: needs (+ lfx-artifact-name: dist-lfx when release_lfx)
end
R->>TCPT: invoke reusable with lfx-artifact-name
opt release_lfx == true
BLFX-->>TLFX: needs
R->>TLFX: matrix (OS x Python), download dist-lfx, run `lfx --help`
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
.github/workflows/cross-platform-test.yml (5)
21-24: Set a default for lfx-artifact-name to avoid null handling quirks.Providing a default empty string makes downstream checks simpler and consistent across workflow_call vs. workflow_dispatch.
lfx-artifact-name: description: "Name of the LFX package artifact" required: false type: string + default: ""
176-189: Simplify wheel selection: let pip resolve instead of find/head.Using a glob lets pip/uv pick the appropriate wheel and avoids accidentally installing the wrong file if multiple wheels exist.
- find ./lfx-dist -name "*.whl" -type f - WHEEL_FILE=$(find ./lfx-dist -name "*.whl" -type f | head -1) - if [ -n "$WHEEL_FILE" ]; then - uv pip install --python ./test-env/Scripts/python.exe "$WHEEL_FILE" - else - echo "No wheel file found in ./lfx-dist/" - exit 1 - fi + shopt -s nullglob + if compgen -G "./lfx-dist/*.whl" > /dev/null; then + uv pip install --python ./test-env/Scripts/python.exe ./lfx-dist/*.whl + else + echo "No wheel file found in ./lfx-dist/"; exit 1 + fi
218-231: Mirror the simplified install on Unix paths.Same rationale as Windows; reduces duplication and error surface.
- find ./lfx-dist -name "*.whl" -type f - WHEEL_FILE=$(find ./lfx-dist -name "*.whl" -type f | head -1) - if [ -n "$WHEEL_FILE" ]; then - uv pip install --python ./test-env/bin/python "$WHEEL_FILE" - else - echo "No wheel file found in ./lfx-dist/" - exit 1 - fi + shopt -s nullglob + if compgen -G "./lfx-dist/*.whl" > /dev/null; then + uv pip install --python ./test-env/bin/python ./lfx-dist/*.whl + else + echo "No wheel file found in ./lfx-dist/"; exit 1 + fi
176-189: Reduce duplication with a single cross-OS install step.Optional: set an env like PYTHON_BIN per-OS and reuse one step for wheel installs (applies to base, main, and LFX). Keeps future changes DRY.
Also applies to: 218-231
464-477: Apply the same pip-glob simplification in experimental job.To stay consistent and avoid first-wheel selection issues.
Also applies to: 506-519
.github/workflows/release_nightly.yml (1)
170-173: Questionable sleep for PyPI propagation.If LFX isn’t built, this 5-minute sleep is skipped correctly; if built, consider polling PyPI for the exact version instead of fixed sleep to shave time.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/cross-platform-test.yml(7 hunks).github/workflows/release.yml(1 hunks).github/workflows/release_nightly.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: create-nightly-tag
🔇 Additional comments (3)
.github/workflows/cross-platform-test.yml (2)
149-155: LGTM: Artifact download gated correctly.Conditional download prevents spurious failures when no LFX artifact is provided.
436-443: LGTM: Experimental matrix now aware of optional LFX artifact..github/workflows/release.yml (1)
218-242: Nice addition: dedicated LFX cross-platform validation.Standalone validation across OS/Python looks good and keeps coupling low with Langflow artifacts.
| needs: [build-nightly-lfx, build-nightly-base, build-nightly-main] | ||
| uses: ./.github/workflows/cross-platform-test.yml | ||
| with: | ||
| base-artifact-name: "dist-nightly-base" | ||
| main-artifact-name: "dist-nightly-main" | ||
| lfx-artifact-name: "dist-nightly-lfx" | ||
|
|
There was a problem hiding this comment.
Blocking dependency: test-cross-platform will be skipped when build-nightly-lfx is not requested.
Because needs includes build-nightly-lfx, the test job gets skipped when LFX is disabled, unintentionally blocking base/main tests.
Fix by removing the LFX job from needs and passing the LFX artifact name conditionally:
test-cross-platform:
name: Test Cross-Platform Installation
- needs: [build-nightly-lfx, build-nightly-base, build-nightly-main]
+ needs: [build-nightly-base, build-nightly-main]
uses: ./.github/workflows/cross-platform-test.yml
with:
base-artifact-name: "dist-nightly-base"
main-artifact-name: "dist-nightly-main"
- lfx-artifact-name: "dist-nightly-lfx"
+ lfx-artifact-name: ${{ inputs.build_lfx == true && 'dist-nightly-lfx' || '' }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| needs: [build-nightly-lfx, build-nightly-base, build-nightly-main] | |
| uses: ./.github/workflows/cross-platform-test.yml | |
| with: | |
| base-artifact-name: "dist-nightly-base" | |
| main-artifact-name: "dist-nightly-main" | |
| lfx-artifact-name: "dist-nightly-lfx" | |
| test-cross-platform: | |
| name: Test Cross-Platform Installation | |
| needs: [build-nightly-base, build-nightly-main] | |
| uses: ./.github/workflows/cross-platform-test.yml | |
| with: | |
| base-artifact-name: "dist-nightly-base" | |
| main-artifact-name: "dist-nightly-main" | |
| lfx-artifact-name: ${{ inputs.build_lfx == true && 'dist-nightly-lfx' || '' }} |
🤖 Prompt for AI Agents
.github/workflows/release_nightly.yml around lines 301 to 307: the job currently
lists build-nightly-lfx in needs which causes the cross-platform test job to be
skipped when LFX is disabled; remove build-nightly-lfx from the needs array and
instead pass the lfx-artifact-name input conditionally (e.g., use a GitHub
Actions expression to set lfx-artifact-name to an empty string or omit it when
LFX is disabled) so the test job no longer depends on the LFX build but still
receives the artifact name when present.
| needs: [build-base, build-main, build-lfx] | ||
| uses: ./.github/workflows/cross-platform-test.yml | ||
| with: | ||
| base-artifact-name: "dist-base" | ||
| main-artifact-name: "dist-main" | ||
| lfx-artifact-name: ${{ inputs.release_lfx == true && 'dist-lfx' || '' }} | ||
|
|
There was a problem hiding this comment.
Blocking dependency: test-cross-platform is tied to build-lfx even when LFX isn’t being released.
When release_lfx is false, build-lfx is skipped, which can skip test-cross-platform and block base/main release flow.
Remove build-lfx from needs; you already pass the LFX artifact name conditionally:
test-cross-platform:
name: Test Cross-Platform Installation
if: inputs.release_package_base == true || inputs.release_package_main == true
- needs: [build-base, build-main, build-lfx]
+ needs: [build-base, build-main]
uses: ./.github/workflows/cross-platform-test.yml
with:
base-artifact-name: "dist-base"
main-artifact-name: "dist-main"
lfx-artifact-name: ${{ inputs.release_lfx == true && 'dist-lfx' || '' }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| needs: [build-base, build-main, build-lfx] | |
| uses: ./.github/workflows/cross-platform-test.yml | |
| with: | |
| base-artifact-name: "dist-base" | |
| main-artifact-name: "dist-main" | |
| lfx-artifact-name: ${{ inputs.release_lfx == true && 'dist-lfx' || '' }} | |
| test-cross-platform: | |
| name: Test Cross-Platform Installation | |
| if: inputs.release_package_base == true || inputs.release_package_main == true | |
| needs: [build-base, build-main] | |
| uses: ./.github/workflows/cross-platform-test.yml | |
| with: | |
| base-artifact-name: "dist-base" | |
| main-artifact-name: "dist-main" | |
| lfx-artifact-name: ${{ inputs.release_lfx == true && 'dist-lfx' || '' }} |
🤖 Prompt for AI Agents
.github/workflows/release.yml around lines 211 to 217: the workflow currently
lists build-lfx in the needs array which forces the cross-platform test job to
depend on build-lfx even when inputs.release_lfx is false; remove build-lfx from
the needs array so test-cross-platform only depends on build-base and
build-main, since the lfx artifact is already passed conditionally via the
with.lfx-artifact-name input; update the needs line to exclude build-lfx and
ensure the conditional artifact string remains unchanged.
Summary by CodeRabbit
New Features
Tests
Chores