Split CI workflows for better parallelization and separation of concerns [EAR-2135] #1222
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Author
|
Not sure what's up with this double compilation in the build wheels: |
Collaborator
Author
|
Seems to be good for a 30second time save every run. and is also a fully human brain generated commit :) |
ianhi
changed the title
This was referenced Sep 11, 2025
paraseba
approved these changes
Sep 12, 2025
.github/workflows/code-quality.yaml
Outdated
|
|
||
| - name: Run clippy linting | ||
| run: | | ||
| just lint -p icechunk -p icechunk-python |
Collaborator
There was a problem hiding this comment.
we should add -p icechunk-macros everywhere too. In fact, I think there is --workspace to do this more maintainable
ianhi
force-pushed
the
ci-split-workflows
branch
from
134c2ef to
38ad79d
Compare
- Created dedicated code-quality.yaml for formatting, linting, and doc tests - Created separate dependency-check.yaml for cargo-deny security checks with weekly scheduling - Updated rust-ci.yaml to focus purely on testing by removing quality checks - Added separate examples step in rust-ci for clearer failure reporting - Enables better parallelization and clearer separation between testing and quality checks 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The Rust linting for icechunk-python is now handled by the dedicated code-quality workflow, which provides more comprehensive coverage by checking both icechunk and icechunk-python packages. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Removed duplicate pre-commit step from python-check workflow - Added comprehensive pre-commit checks to code-quality workflow using existing .pre-commit-config.yaml - Skips Rust hooks in pre-commit since they're handled separately with just commands - Now covers all quality checks: Python formatting/linting, file checks, actionlint, codespell, etc. - Maintains complete quality coverage while properly separating concerns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Added UV setup with caching and proper working directory - Use uv sync to create environment and install base dependencies - Install built wheel with test dependencies using UV pip - Run pytest with UV run to use the managed environment - Fixes "pytest: command not found" error by ensuring proper installation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Create shared build-wheels job to eliminate duplicate wheel building - Split test and xarray-backends jobs to run in parallel after build - Add uv package manager to all Python jobs for consistency - Separate install and test steps for better debugging - Both test jobs now use artifacts from shared build step 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
ianhi
force-pushed
the
ci-split-workflows
branch
from
38ad79d to
2828477
Compare
Collaborator
Author
|
This is ready now! I will merge in the morning when I have more brainpower. So I can immediately follow up and approriately modify the branch protection rules |
Collaborator
perfect, lmk if you need help merging because of that pending status |
Collaborator
Author
I seem to be ok!
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
General Organization
For both the rust and python CI i have extracted the code quality checks (cargo fmt, pre-commit, etc...) into separate workflows. And the main CI pathways just run compilations and tests.
Use
uvas much as possible when interacting with pythonuse
sccacheon the maturin build stepThe two python steps now share a single
build-wheelstepFixed MinIO/Azurite health checks in both python-check and rust-ci
Potential future improvement
Right now the maturin build doesn't benefit at all from the fact that we are already building the icechunk rust library separately. It feels like this should be possible, but I couldn't figure it out
Current status
I think this is basically ready. It seems to significantly speed things up. For example the python tests go from 8 minutes on a PR from before this to ~5 min on this PR. The rust ci checks are also sped up.
My one worry is that the tests that normally fail on PR due to credentials are not failing here. But I think that might be because I gained some icechunk status on github.
After merging this we will need to edit the names of the required workflows for merging. That's the cause of this
it's waiting for a workflow that no longer exists. We will just update that to be the "tests" or "xarray-tests" jobs