-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[Replace #2507] Support VSTS and other custom registries with unusual feeds #3231
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
|
Would be great to get some feedback on this! 🙂 |
|
Great job, thanks for patience, @lumaxis. |
|
@lumaxis, now that we have this feature in Yarn it would be great for people to know about it. |
|
Has this feature been released in a public downloadable yarn ? I so, which version? |
|
Not yet, will be released in 0.26 |
|
@bestander I think I should be able to put something together within the week 🙂 Anything special I should look out for? |
|
Cool! |
|
@bestander There you go 🙂 |
|
@lumaxis, this change might have caused #3765. Apparently we don't have tests covering neither private repos nor vsts repo and I am concerned that change in one might break the other. |
|
@bestander Oh dear, that sounds terrible! Very sorry to maybe have caused that 😞 |
|
Thanks, Lukas.
Very much appreciated, it could be not your change after all but it seems
to touch related code.
It is our (mine) laziness to blame though, we should have set up e2e tests
for private packages long ago.
…On Fri, Jun 30, 2017 at 12:48 PM Lukas Spieß ***@***.***> wrote:
@bestander <https://github.com/bestander> Oh dear, that sounds terrible!
Very sorry to maybe have caused that 😞
I was kind of expecting that private repos were covered by either unit or
integration tests already, so didn't check that explicitly ...
I'll take a look at #3774 <#3774> and
also check if and how we could set up an E2E test.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3231 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACBdWMZ5l3Wt5F52iy4MtAKPHVHE8i7dks5sJVDxgaJpZM4NFNVM>
.
|
|
We had an issue where we would get 401 with VSTS npm feed, yielded the following error for .npmrc was like this setting the URL to VSTS have updated the URL generated in the instructions so it now produces them with the trailing slash. ~~~So this should not be a problem in the future, but this tripped us up this morning. Not sure why this would be an issue. Worth noting that it works fine with, and without, trailing slash with Just realised what the actual problem was, will open a separate issue #4413 |
This is a continuation of #2507 where the progress has recently stalled. Fixes #2505.
Quick Summary
Visual Studio Team Services (VSTS) unfortunately produces package feed URLs that look like
https://account.pkgs.visualstudio.com/_packaging/feed-name/npm/registrybut the URLs to fetch the actual tars then point tohttps://account.pkgs.visualstudio.com/_packaging/a917cf10-dfe4-4781-b0d1-105df772da02/npm/registry/@scope/package-name/-/package-name-0.0.1.tgz.This Pull Request aims to fix this by picking up a suggestion from #2507 by @bestander and introduces a new
.npmrcoption calledcustom-host-suffix. That options allows to skip a check in the form ofrequestPath.startsWith(registryPath)if the request's URL contains the value ofcustom-host-suffix.Tests
I tried to add tests as best as I could but had trouble coming up with a good way of integration testing this change as I couldn't find an easy way to set up a package feed on VSTS that is publicly accessible. Is there a precedence for using shared credentials in the test suite? Maybe that would be an option.
I'll appreciate any advice for better tests 👍