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

Update NuGet.config in HttpStress #55519

Merged
merged 2 commits into from
Jul 12, 2021
Merged

Update NuGet.config in HttpStress #55519

merged 2 commits into from
Jul 12, 2021

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Jul 12, 2021

We can't use nuget.org in this manner to comply with security guidance.

We can't use nuget.org in this manner to comply with security guidance
@ghost
Copy link

ghost commented Jul 12, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

We can't use nuget.org in this manner to comply with security guidance.

Author: hoyosjs
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

Thank you!

@hoyosjs
Copy link
Member Author

hoyosjs commented Jul 12, 2021

Once the HttpStress tests build I'll merge this to unblock the JIT rolling builds.

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

I think this will break our stress pipeline. Could we run it before we merge?

@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<configuration>
<packageSources>
<!-- Add public nuget feed. -->
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
<clear />
Copy link
Member

Choose a reason for hiding this comment

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

Why do you clear this? We need what's in repository wide nuget.config

Copy link
Member Author

Choose a reason for hiding this comment

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

If this is true, then you should not have this file at all. We don't allow nuget.org. Let me delete this and we'll see the fallout. FYI, this already broke other partners within the runtime repo.

Copy link
Member

Choose a reason for hiding this comment

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

Let's try. BTW, /azp run runtime-libraries stress-http is the magic to run the pipeline.

Copy link
Member

Choose a reason for hiding this comment

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

One more thing, can we import packages from nuget.org to dotnet-public if they are missing? What's the process for that?

<!-- Add public nuget feed. -->
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
<clear />
<add key="dotnet-public" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public/nuget/v3/index.json" />
Copy link
Member

Choose a reason for hiding this comment

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

Will all the packages we use be there? Could you run the stress pipeline on this PR? It's not part of inner loop.

@ManickaP
Copy link
Member

/azp run runtime-libraries stress-http

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ManickaP
Copy link
Member

/azp run runtime-libraries stress-http

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

The http stress pipeline run for the last change.
I don't want to wait for it, so approving, thanks.

@ManickaP
Copy link
Member

The pipeline looks good :shipit:

@hoyosjs hoyosjs merged commit efd0bb5 into main Jul 12, 2021
@hoyosjs hoyosjs deleted the juhoyosa/fix-nuget branch July 12, 2021 19:24
@karelz karelz added this to the 6.0.0 milestone Jul 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants