Skip to content
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

fix(ci): Bring bundle-size-comparison back online #19638

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

alexvy86
Copy link
Contributor

@alexvy86 alexvy86 commented Feb 15, 2024

Description

The azure-devops-node-api package our build-tools leverage to download pipeline artifacts with baseline bundle size measurements has a known issue when trying to download artifacts created with the newer PublishPipelineArtifact task in ADO, which we are forced to use since we updated the pipelines to use 1ES pipeline templates (it worked before with the PublishBuildArtifacts task).

This PR implements a workaround suggested in the thread linked above to get the download to work.

Reviewer Guidance

The review process is outlined on this wiki page.

Confirmed that the fix works by running danger locally.

The original version of the function that the hack overrides can be seen here.

@github-actions github-actions bot added area: build Build related issues base: main PRs targeted against main branch labels Feb 15, 2024
@alexvy86 alexvy86 requested a review from a team February 15, 2024 16:52
@alexvy86 alexvy86 merged commit 337fcd2 into microsoft:main Feb 15, 2024
29 checks passed
@alexvy86 alexvy86 deleted the fix-bundle-size branch February 15, 2024 16:58
// The workaround is to override the createAcceptHeader function when making the request to download the artifact.
// See https://github.com/microsoft/azure-devops-node-api/issues/432 for more details
const originalCreateAcceptHeader = buildApi.createAcceptHeader;
buildApi.createAcceptHeader = (type: string): string => type;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The comment in the GH issue is helpful for understanding what the workaround does and why it's necessary. Perhaps replicate it here? (slightly sanitized).

// ADO's APIs try to download pipeline artifacts using an API version that isn't supported
// by the artifact download endpoint.
// One way of getting around that is by temporarily removing the API version to force it to use
// a supported version.
// See: https://github.com/microsoft/azure-devops-node-api/issues/432

The historical information (used to work, 1ES forced our hand, etc.) might be better expressed in the PR comments, since it's explaining the motivation for the delta, not the motivation for the code.

@alexvy86 alexvy86 mentioned this pull request Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants