Speed up Windows CI setup#3565
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe ChangesCI Workflow Update
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
|
📚 Docs Preview: https://pr-3565.datamodel-code-generator.pages.dev |
Merging this PR will improve performance by 17.59%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_perf_multiple_files_input |
3.6 s | 3 s | +20.41% |
| ⚡ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.6 s | 2.2 s | +18.56% |
| ⚡ | WallTime | test_perf_graphql_style_pydantic_v2 |
802.1 ms | 677.1 ms | +18.46% |
| ⚡ | WallTime | test_perf_stripe_style_pydantic_v2 |
2 s | 1.7 s | +18.37% |
| ⚡ | WallTime | test_perf_duplicate_names |
1,057.6 ms | 895.5 ms | +18.11% |
| ⚡ | WallTime | test_perf_deep_nested |
5.2 s | 4.4 s | +17.55% |
| ⚡ | WallTime | test_perf_all_options_enabled |
5.4 s | 4.6 s | +17.52% |
| ⚡ | WallTime | test_perf_openapi_large |
3 s | 2.5 s | +17.33% |
| ⚡ | WallTime | test_perf_large_models_pydantic_v2 |
3.5 s | 3 s | +16.64% |
| ⚡ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.9 s | 1.7 s | +15.48% |
| ⚡ | WallTime | test_perf_complex_refs |
2 s | 1.7 s | +15.1% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ci/speed-up-windows-setup (14d9007) with main (a3de5a0)
Footnotes
-
98 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3565 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 163 163
Lines 34737 34737
Branches 4031 4031
=========================================
Hits 34737 34737
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/test.yaml (1)
400-422: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a single source of truth for the pinned versions.
0.11.26,4.56.1, and1.35.2are each hardcoded independently in theversion:input, thecache-suffix, and theuv tool installcommand. A future version bump that misses one of these spots would silently leave a stalecache-suffix(harmless functionally, since uv's cache format tolerates cross-version sharing, but it defeats the intended cache-key precision and can mask which uv/tox/tox-uv combo actually populated a given cache).♻️ Suggested consolidation via job-level env vars
+ env: + OS: ${{ matrix.os }} + SHARD_INDEX: ${{ matrix.shard }} + SHARD_TOTAL: ${{ matrix.shard_total }} + TOX_ENV: ${{ matrix.tox_env }} + WINDOWS_UV_VERSION: "0.11.26" + WINDOWS_TOX_VERSION: "4.56.1" + WINDOWS_TOXUV_VERSION: "1.35.2" steps: - name: Install uv (Windows) if: matrix.os == 'windows-latest' uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 with: - version: "0.11.26" + version: "${{ env.WINDOWS_UV_VERSION }}" ... - cache-suffix: "windows-${{ matrix.tox_env }}-uv0.11.26-tox4.56.1-toxuv1.35.2" + cache-suffix: "windows-${{ matrix.tox_env }}-uv${{ env.WINDOWS_UV_VERSION }}-tox${{ env.WINDOWS_TOX_VERSION }}-toxuv${{ env.WINDOWS_TOXUV_VERSION }}" - name: Install tox (Windows) if: matrix.os == 'windows-latest' - run: uv tool install --python-preference only-managed --python 3.13 "tox==4.56.1" --with "tox-uv==1.35.2" + run: uv tool install --python-preference only-managed --python 3.13 "tox==${{ env.WINDOWS_TOX_VERSION }}" --with "tox-uv==${{ env.WINDOWS_TOXUV_VERSION }}"🤖 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/test.yaml around lines 400 - 422, The Windows uv/tox versions are duplicated across the workflow, so update the Install uv (Windows), cache-suffix, and Install tox (Windows) steps to read from a single pinned source instead of hardcoding 0.11.26, 4.56.1, and 1.35.2 in multiple places. Use a shared job-level environment or equivalent constants so the astral-sh/setup-uv and uv tool install references stay in sync and any future bump automatically updates the cache suffix too.
🤖 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/test.yaml:
- Around line 414-419: The uv cache key is currently shard-specific in the
workflow’s setup-uv step, which prevents non-Windows shards from reusing each
other’s cache. Update the cache suffix used in the “Install the latest version
of uv” step to be shared across shards, and ensure only one shard is responsible
for saving the cache, following the same pattern used for Windows so all shards
can restore the same dependencies.
---
Nitpick comments:
In @.github/workflows/test.yaml:
- Around line 400-422: The Windows uv/tox versions are duplicated across the
workflow, so update the Install uv (Windows), cache-suffix, and Install tox
(Windows) steps to read from a single pinned source instead of hardcoding
0.11.26, 4.56.1, and 1.35.2 in multiple places. Use a shared job-level
environment or equivalent constants so the astral-sh/setup-uv and uv tool
install references stay in sync and any future bump automatically updates the
cache suffix too.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c88c0fe5-f1e1-4a56-a71b-1577fd6aafcc
📒 Files selected for processing (1)
.github/workflows/test.yaml
…etup # Conflicts: # .github/workflows/test.yaml
Breaking Change AnalysisResult: No breaking changes detected Reasoning: PR #3565 changes only the CI workflow file (.github/workflows/test.yaml) to speed up Windows CI setup — pinning uv/tox/tox-uv versions for Windows, adding caching, and adjusting a cache-suffix. It touches no library source code, generated output, Jinja2 templates, CLI options, Python API, defaults, supported Python versions, or error handling. The only deleted line is a cache-suffix string in CI config. Nothing affects end users of datamodel-code-generator, so there are no breaking changes. This analysis was performed by Claude Code Action |
|
🎉 Released in 0.68.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
Summary by CodeRabbit