Skip to content

Conversation

@mbg
Copy link
Member

@mbg mbg commented Jan 24, 2026

On Windows, choco install yq routinely 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 install just queries a feed at https://community.chocolatey.org/api/v2/ for where to download a suitable binary for yq from, which turns out to be a GitHub release.

This PR changes the workflow to sidestep choco entirely and just download yq directly from the release that's currently used by choco.

Note for reviewers: in the first commit, I attempted to cache the data for choco install instead, but I discovered that choco doesn'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:

  • Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Environments:

  • Testing/None - This change does not impact any CodeQL workflows in production.

How did/will you validate this change?

  • End-to-end tests - I am depending on PR checks (i.e. tests in pr-checks).

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

  • Rollback - Change can only be disabled by rolling back the release or releasing a new version with a fix.

How will you know if something goes wrong after this change is released?

  • Other - Please provide details. CI outcome.

Are there any special considerations for merging or releasing this change?

  • No special considerations - This change can be merged at any time.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Consider adding a changelog entry for this change.
  • Confirm the readme and docs have been updated if necessary.

@github-actions github-actions bot added the size/S Should be easy to review label Jan 24, 2026
@github-actions github-actions bot added size/XS Should be very easy to review and removed size/S Should be easy to review labels Jan 24, 2026
@mbg mbg changed the title Add installYq option to sync.py and cache downloads Add installYq option to sync.py and install yq directly from GitHub release Jan 24, 2026
@mbg mbg marked this pull request as ready for review January 24, 2026 14:17
@mbg mbg requested a review from a team as a code owner January 24, 2026 14:17
Copilot AI review requested due to automatic review settings January 24, 2026 14:17
Copy link
Contributor

Copilot AI left a 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 installYq option to sync.py that generates a step to download yq directly from GitHub releases on Windows
  • Updated build-mode-autobuild.yml to use the new installYq: "true" option instead of manually installing via choco

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(
Copy link

Copilot AI Jan 24, 2026

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).

Suggested change
'run': LiteralScalarString(
'run': LiteralScalarString(
'mkdir -p "$YQ_PATH"\n'

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@esbena esbena left a 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?

'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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'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?

Copy link
Member Author

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 \n is 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.

@mbg
Copy link
Member Author

mbg commented Jan 26, 2026

Do you happen to know why this isn't done through https://github.com/frenck/action-setup-yq (or similar)

I suppose it's to avoid adding extra layers of dependencies, particularly on third-party actions.

so dependabot would have a theoretical chance?

Dependabot would be able to update the (new) third-party action, but not the version of yq we use. If we don't add an extra dependency here, then there's nothing for Dependabot to do.

Maybe if we stuck with choco and pinned a particular version of yq in a config file, Dependabot would be able to update that for us, but we previously let choco just pull whatever (latest) version it wanted anyway.


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.

Copy link
Contributor

@esbena esbena left a 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.

@mbg mbg enabled auto-merge January 26, 2026 11:04
@mbg mbg merged commit 25a224b into main Jan 26, 2026
250 checks passed
@mbg mbg deleted the mbg/ci/yq-windows branch January 26, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Should be very easy to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants