-
Notifications
You must be signed in to change notification settings - Fork 433
Add installYq option to sync.py and install yq directly from GitHub release
#3423
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
installYq option to sync.py and cache downloadsinstallYq option to sync.py and install yq directly from GitHub release
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 addresses reliability issues with choco install yq on Windows by replacing it with direct downloads from the GitHub release. The change sidesteps Chocolatey's feed query failures and downloads the yq binary directly from the same GitHub release that Chocolatey would use.
Changes:
- Added
installYqoption tosync.pythat generates a step to downloadyqdirectly from GitHub releases on Windows - Updated
build-mode-autobuild.ymlto use the newinstallYq: "true"option instead of manually installing viachoco
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pr-checks/sync.py | Adds installYq option that generates a workflow step to download yq v4.50.1 from GitHub releases on Windows runners |
| pr-checks/checks/build-mode-autobuild.yml | Replaces manual choco install yq step with declarative installYq: "true" option |
| .github/workflows/__build-mode-autobuild.yml | Auto-generated workflow file (not reviewed per guidelines) |
| 'env': { | ||
| 'YQ_PATH': '${{ runner.temp }}/yq' | ||
| }, | ||
| 'run': LiteralScalarString( |
Copilot
AI
Jan 24, 2026
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.
The directory $YQ_PATH should be created before downloading the file to ensure it exists. Consider adding mkdir -p "$YQ_PATH" as the first line of the run script to be consistent with similar patterns elsewhere in the codebase (e.g., .github/workflows/__cleanup-db-cluster-dir.yml:60).
| 'run': LiteralScalarString( | |
| 'run': LiteralScalarString( | |
| 'mkdir -p "$YQ_PATH"\n' |
esbena
left a comment
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.
LGTM.
Do you happen to know why this isn't done through https://github.com/frenck/action-setup-yq (or similar) so dependabot would have a theoretical chance?
pr-checks/sync.py
Outdated
| 'YQ_PATH': '${{ runner.temp }}/yq' | ||
| }, | ||
| 'run': LiteralScalarString( | ||
| 'gh release download --repo mikefarah/yq --pattern "yq_windows_amd64.exe" v4.50.1 -O "$YQ_PATH/yq.exe"\n' |
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.
| 'gh release download --repo mikefarah/yq --pattern "yq_windows_amd64.exe" v4.50.1 -O "$YQ_PATH/yq.exe"\n' | |
| 'gh release download --repo mikefarah/yq --pattern "yq_windows_amd64.exe" "$YQ_VERSION" -O "$YQ_PATH/yq.exe"' |
I kind of agree with copilot.
Let's move the version out to a variable, and add and add an explanation and a direct link to the release as a comment.
Also, the trailing \n is confusing to read. Could we do something cleaner syntactically, or is this a workflow builder limitation?
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.
Let's move the version out to a variable, and add and add an explanation and a direct link to the release as a comment.
Sure, I can do that.
Also, the trailing
\nis confusing to read. Could we do something cleaner syntactically, or is this a workflow builder limitation?
There might be something that's cleaner syntactically in Python, but this was the sanest option I came up with since Python multi-line strings mostly led to results with weird spacing. I am open to suggestions.
I suppose it's to avoid adding extra layers of dependencies, particularly on third-party actions.
Dependabot would be able to update the (new) third-party action, but not the version of Maybe if we stuck with I think the bottom line here is that this is a minor tool dependency that (ideally) would come pre-installed on Windows runners. This is the only workflow that currently runs on Windows and needs it, and we only have a very basic use for it. I don't think we'd want to add extra maintenance overhead or a particularly complex solution to maintain this. The main goal of this PR is just to avoid the intermittent workflow failures. |
esbena
left a comment
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.
Thanks for elaborating and improving.
On Windows,
choco install yqroutinely fails. For some reason, this doesn't cause the step itself to fail, but does cause obvious failures later on in the workflow.Under the hood,
choco installjust queries a feed athttps://community.chocolatey.org/api/v2/for where to download a suitable binary foryqfrom, which turns out to be a GitHub release.This PR changes the workflow to sidestep
chocoentirely and just downloadyqdirectly from the release that's currently used bychoco.Note for reviewers: in the first commit, I attempted to cache the data for
choco installinstead, but I discovered thatchocodoesn't really support caching to the point where it doesn't have to query the feed / redownloads the files.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Environments:
How did/will you validate this change?
pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist