-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CI: Increase uv timeout and reduce deps install #146
Conversation
WalkthroughThe pull request updates the GitHub Actions workflows for building documentation, releasing packages, and testing. Key modifications include simplifying the Python version matrix to only "3.13", adding an environment variable Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #146 +/- ##
==========================================
- Coverage 98.31% 98.15% -0.16%
==========================================
Files 15 12 -3
Lines 1008 975 -33
==========================================
- Hits 991 957 -34
- Misses 17 18 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/build_docs.yml (1)
Line range hint
1-87
: Consider alternative approaches to speed up doc buildsWhile the current changes achieve faster builds by reducing matrix combinations and dependencies, consider these alternative approaches that maintain compatibility:
- Use dependency caching more effectively
- Parallelize the documentation build process
- Use a stable Python version but optimize the build steps
- Consider using pre-built documentation images
This would achieve the speed objective without compromising the testing coverage across Python versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/build_docs.yml
(2 hunks)
🔇 Additional comments (3)
.github/workflows/build_docs.yml (3)
45-45
: Verify documentation dependencies are complete
The change from --all-extras --dev
to --extra docs
will make installation faster. However, we should verify that all required documentation dependencies are properly specified in the docs
extra.
#!/bin/bash
# Check docs extra dependencies in pyproject.toml
echo "Checking docs dependencies..."
rg -A 10 '\[project\.optional-dependencies\]' pyproject.toml
# Check if any documentation imports might be missing from docs extra
echo "Checking doc-related imports in documentation source..."
rg -t python "^import|^from.*import" docs/
Line range hint 74-76
: LGTM: Good practice using pyproject.toml for version specification
Using python-version-file: "pyproject.toml"
is a good practice as it centralizes Python version requirements in one place.
#!/bin/bash
# Verify Python version specification in pyproject.toml
echo "Checking Python version specification..."
rg "python_version|requires-python" pyproject.toml
Line range hint 13-21
: Reconsider using only Python 3.13 for documentation builds
While reducing the matrix combinations will make the CI faster, using only Python 3.13 (which is still in development) for documentation builds could be risky:
- Python 3.13 is not yet stable
- Documentation might have compatibility issues with older Python versions that users are more likely to use
- Cross-platform issues might go undetected with Ubuntu-only testing
Consider keeping at least one stable Python version (e.g., 3.12) in the matrix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/release.yml (1)
Line range hint
1-35
: Consider documenting the changes in PR description.The workflow changes look good overall, but it would be helpful to:
- Fix the typo in PR title from "Ci/doc build fater" to "Ci/doc build faster"
- Add a PR description explaining the motivation for adding the timeout and any observed issues it addresses
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/build_docs.yml
(3 hunks).github/workflows/release.yml
(1 hunks).github/workflows/test.yml
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build_docs.yml
🔇 Additional comments (3)
.github/workflows/release.yml (1)
10-11
: LGTM! Good addition of timeout configuration.
Setting a 5-minute timeout for dependency installation is a reasonable default and helps prevent hanging builds. The consistent application across workflows is a good practice.
Let's verify the timeout setting in other workflow files:
✅ Verification successful
Timeout configuration is consistently set across all workflow files ✅
The verification confirms that the UV_HTTP_TIMEOUT: 300
environment variable is consistently configured in all three relevant workflow files:
.github/workflows/test.yml
.github/workflows/release.yml
.github/workflows/build_docs.yml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the same timeout is configured in other workflow files
rg "UV_HTTP_TIMEOUT: 300" .github/workflows/
Length of output: 298
.github/workflows/test.yml (2)
34-34
: Verify test dependencies coverage with specific extras.
The change from --all-extras --dev
to --extra dev --extra plot
makes the dependency installation more selective, which could improve build times. However, we need to ensure all required dependencies for testing are included.
Let's verify the dependencies:
#!/bin/bash
# Description: Check if all test dependencies are covered by the selected extras
# Expected: All imports in test files should be satisfied by the selected extras
# Check imports in test files
rg -t python "^import|^from.*import" tests/
# Check extras defined in pyproject.toml
rg "^\[project.optional-dependencies\]" -A 20 pyproject.toml
13-14
: Document the reason for the timeout value.
While adding a timeout for dependency installation is good practice, it would be helpful to document why 5 minutes was chosen as the threshold. Consider whether this timeout is sufficient across all Python versions in the matrix.
Let's verify if this timeout is consistently set across all workflow files:
✅ Verification successful
The timeout value is consistently set across all workflow files
The 5-minute timeout value (300 seconds) is consistently defined in all GitHub Actions workflow files where dependency installation occurs:
.github/workflows/test.yml
.github/workflows/build_docs.yml
.github/workflows/release.yml
Each file includes the same comment explaining the timeout duration. While documenting more details about why this specific value was chosen could be helpful, the consistency across workflows suggests this is a well-considered configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for UV_HTTP_TIMEOUT consistency across workflow files
# Expected: All workflow files should have the same timeout value
rg -A 2 "UV_HTTP_TIMEOUT" .github/workflows/
Length of output: 505
Script:
#!/bin/bash
# Let's try again with the correct command to search workflow files
rg -A 2 "UV_HTTP_TIMEOUT" .github/workflows/
Length of output: 591
Summary by CodeRabbit
UV_HTTP_TIMEOUT
to enhance dependency installation timeout.