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

Use CFS rather than public npm registry in PR builds and official builds #9837

Merged
merged 7 commits into from
Feb 7, 2025

Conversation

debonte
Copy link
Collaborator

@debonte debonte commented Feb 6, 2025

This is a replacement for #9243. In that original PR, I was checking in .npmrc files which forced all builds (including builds on dev machines) to target Pyright's Azure Artifacts feed. In this PR, those .npmrc files are generated during PR builds and official builds. in case you're wondering -- PR builds use the CFS feed because if CFS rejects a particular package dependency, we want to find out about that when the dependency is added rather than when we're trying to ship a release. The behavior of other build types (ex. dev machine builds) is unchanged; they will continue to target the public npm registry.

In official builds on AzDO, the .npmrc files are generated by part of the pipeline that is owned by the Azure Artifacts team. This behavior is triggered by the existence of the AZURE_ARTIFACTS_FEED variable in azure-pipelines-release.yml. This is a new feature that they recently shipped.

In GitHub PR builds, the .npmrc files are generated by a custom action (cfs-npm-authenticate). The build authenticates with the Azure Artifacts feed using WIF. I'm expecting the initial PR validation build for this PR to fail because I haven't granted the pyright repo access to the relevant service principal yet. After the initial build fails, I will grant that access and verify that the PR build succeeds.

Fix https://github.com/microsoft/pyrx/issues/6252

@debonte debonte requested review from bschnurr and erictraut February 6, 2025 23:41
@erictraut
Copy link
Collaborator

Thanks @debonte. I don't know enough about the CFS feed to effectively review this change. If you're happy with it and it seems to work, go ahead and merge it.

@debonte debonte merged commit f749f6b into main Feb 7, 2025
10 checks passed
@debonte debonte deleted the debonte/centralFeedService branch February 7, 2025 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants