-
Notifications
You must be signed in to change notification settings - Fork 277
ci: fix cirrus and reduce rebuilds #2460
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
Conversation
e7fdb15
to
a5be50f
Compare
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
a5be50f
to
c481bf5
Compare
The last build before "Monthly commute limit exceeded" did pass on Windows! Just in time. |
@joerick, what about turning off builds on main for some CIs like Cirrus? They use a lot of credits (every PR merge), and aren't really very helpful. |
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.
Pull Request Overview
This PR aims to reduce unnecessary rebuilds and fix Cirrus CI GraalPy by updating CI configuration files and adding additional checks.
- Added a warm-up socket connection in test_ssl.py to mitigate connection delays during SSL tests.
- Updated certificate installation commands in Windows tasks and refined file exclusion patterns across CI configurations.
- Modified the 'only_if' condition in .cirrus.yml to better control rebuild triggers.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
test/test_ssl.py | Added socket warm-up command to improve SSL test reliability. |
examples/cirrus-ci-minimal.yml | Introduced certificate generation commands in Windows tasks. |
azure-pipelines.yml | Expanded file exclusion list to prevent redundant pipeline runs. |
.github/workflows/test.yml | Added extra files to paths-ignore for more efficient CI triggering. |
.cirrus.yml | Updated the only_if logic and certificate commands for better clarity. |
only_if: changesInclude('.cirrus.yml') || ($CIRRUS_BRANCH == "main" && !changesIncludeOnly('.github/*', 'bin/*', 'docs/*', '.circleci/*', '.travis.yml', '.pre-commit-config.yaml', '.readthedocs.yml', 'azure-pipelines.yml', 'README.md', 'mkdocs.yml', 'noxfile.py')) || $CIRRUS_BRANCH =~ 'cirrus.*' | ||
|
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.
[nitpick] The current 'only_if' condition is complex and may be difficult to maintain. Consider refactoring the logic or extracting the file patterns into a variable to improve readability and ease of future modifications.
only_if: changesInclude('.cirrus.yml') || ($CIRRUS_BRANCH == "main" && !changesIncludeOnly('.github/*', 'bin/*', 'docs/*', '.circleci/*', '.travis.yml', '.pre-commit-config.yaml', '.readthedocs.yml', 'azure-pipelines.yml', 'README.md', 'mkdocs.yml', 'noxfile.py')) || $CIRRUS_BRANCH =~ 'cirrus.*' | |
IGNORED_FILE_PATTERNS: &IGNORED_FILE_PATTERNS | |
- .github/* | |
- bin/* | |
- docs/* | |
- .circleci/* | |
- .travis.yml | |
- .pre-commit-config.yaml | |
- .readthedocs.yml | |
- azure-pipelines.yml | |
- README.md | |
- mkdocs.yml | |
- noxfile.py | |
only_if: changesInclude('.cirrus.yml') || ($CIRRUS_BRANCH == "main" && !changesIncludeOnly(*IGNORED_FILE_PATTERNS)) || $CIRRUS_BRANCH =~ 'cirrus.*' |
Copilot uses AI. Check for mistakes.
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.
I can't check to see if this works, so fine to skip for now, though it does look better. Hopefully someone remembers to try it next month.
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.
Looks good to me!
Cirrus currently only builds on |
What about having it only rebuild on the dependency update PRs? That's 1-2 per week. |
Yeah, seeing as we're hitting rate limits on main, that's a nice idea.
|
Reducing rebuilds when unrelated files change.
Trying to fix Cirrus CI GraalPy.