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

Fixes #4405 About publishing nugets to public feed #4406

Merged
merged 2 commits into from
Oct 29, 2019

Conversation

antoniovs1029
Copy link
Member

@antoniovs1029 antoniovs1029 commented Oct 29, 2019

Fixes #4405 about publishing nugets to this public feed:
https://dev.azure.com/dnceng/public/_packaging?_a=feed&feed=MachineLearning

This will be executed by the AzureDevOps build pipeline whenever a new commit is added to the master branch of this repo. Notice that sometimes there are some problems on the side of Azure DevOps, and it might fail when executing the build pipeline, even in steps that were not modified in this PR, and producing errors that prevent the pipeline to actually publish the nugets to the feed; this unplanned errors already existed before the changes introduced in this PR, and they are somewhat unpredictable. The solution to this is to rerun the build manually until the pipeline succeeds.

Worked this out following @safern instructions.

@antoniovs1029 antoniovs1029 requested a review from a team as a code owner October 29, 2019 00:13
@@ -174,13 +174,13 @@ phases:
variables:
BuildConfig: Release
OfficialBuildId: $(BUILD.BUILDNUMBER)
DOTNET_CLI_TELEMETRY_OPTOUT: 1
DOTNET_CLI_TELEMETRY_OUTPUT: 1
Copy link
Member Author

@antoniovs1029 antoniovs1029 Oct 29, 2019

Choose a reason for hiding this comment

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

@safern told me this was a typo that needed to be fixed. I believe this change doesn't affect directly the issue that is being fixed in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Changing this was a mistake. If you search for DOTNET_CLI_TELEMETRY_OUTPUT in all of github, you will find this is the only place this string is used. No one reads an OUTPUT environment variable.

But if you search for DOTNET_CLI_TELEMETRY_OPTOUT you will see it is used in hundreds of places.

Note: OPTOUT stands for "opt out" - as in "don't log CLI telemetry".

Copy link
Member

Choose a reason for hiding this comment

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

You’re completely right I really got confused by reading output and just seems wrong and I recommended to change, my bad @antoniovs1029

Copy link
Member

@codemzs codemzs left a comment

Choose a reason for hiding this comment

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

:shipit:

packagesToPush: $(Build.SourcesDirectory)/bin/packages/**/*.nupkg;!$(Build.SourcesDirectory)/bin/packages/**/*.symbols.nupkg
nuGetFeedType: internal
feedPublish: MachineLearning

# - task: MSBuild@1
Copy link
Member Author

@antoniovs1029 antoniovs1029 Oct 29, 2019

Choose a reason for hiding this comment

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

@codemzs I wonder if I should delete this commented task, since I believe this won't be used in the future?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please remove.

@@ -226,21 +226,20 @@ phases:
msbuildVersion: 15.0
continueOnError: false

- task: NuGetCommand@2
Copy link
Member Author

Choose a reason for hiding this comment

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

I asked @codemzs and he told me he was okay with me removing this task that publishes the nugets into the private feed. Still, he was concerned about not knowing if this would "break any dependencies". It seems it doesn't, but if anyone has a reason for not removing this task, please tell me. (@eerhardt ?).

Copy link
Member Author

@antoniovs1029 antoniovs1029 Oct 29, 2019

Choose a reason for hiding this comment

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

So after I mentioned to @codemzs that publishing to the public feed might not always automatically work because of problems on the side of AzureDevOps, he recommended me to put back the task that publishes to the private feed.

Still, I decided to put it back after the other publishing tasks, since if the task of publishing to the private feed fails it would interrupt the other publishing tasks.

If you want to see it before the task of publishing to the public feed, please tell me.

Notice that having both tasks (publishing both to the public and to the private feed) increases the chances of having a failure on Azure DevOps side, making it more likely for the pipeline to fail.

Copy link
Member

Choose a reason for hiding this comment

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

So after I mentioned to @codemzs that publishing to the public feed might not always automatically work because of problems on the side of AzureDevOps

This is not publishing to the public feed itself, the build definition already has that flakiness because of network connection issues or other unrelated issues to publishing... I think we shouldn't be publishing to both sides as it just slows down builds, it's expensive and nobody will be looking at the internal feed.

Copy link
Member Author

@antoniovs1029 antoniovs1029 Oct 29, 2019

Choose a reason for hiding this comment

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

I agree that the problem isn't in the publishing step itself. And I also agree that maybe it's better to not have the step that publishes to the private feed.

I've looked into the builds that were executed in the private pipelines before this PR, and it seems to me that in many cases the nugets weren't even published to the private feed because something went wrong with the pipeline.

After this PR got merged, and after Maryam's latest PR got merged an hour later, their respective automatic builds failed. I've also tried to manually rerun the builds, but they also fail. The failures occur in steps of the pipeline that execute even before trying to publish to the public feed, so this failures are not related to the changes I added in this PR; for example, they fail while installing a plug in, or while signing builds.

So, so far, the things I've introduced in this PR haven't been actually executed in Azure DevOps in the master branch (I've only tested my changes by running them from a feature branch in the private repo, and that worked as expected and published the nugets to the feed, but even then it required me to run a couple of manual builds because of the same kind of problems I've already described).

I am somewhat worried about all of these, because it seems that other people will rely on the public feed to be always up-to-date (like @frank-dong-ms nightly build tests). But I wouldn't know if there could be other options, or workarounds to all of this... or if this actually poses a problem to them.

Copy link
Member Author

@antoniovs1029 antoniovs1029 Oct 29, 2019

Choose a reason for hiding this comment

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

Ok, so in the last two builds (one manual, and one triggered automatically after @codemzs latest PR), the task of publishing to the public feed ran successfully, but the pipeline actually failed when publishing to the private feed (and the errors in both cases were different, so my guess is that this was, again, caused by an error on Azure Devops side).

So at least the changes I've made in this PR seem to be working for publishing to the public feed. It's just that it might require some luck for the whole pipeline to run without problems.

Notice, though, that even if the pipeline says that the nugets were published successfully to the public feed after Zeeshan's PR got merged 3 hours ago, if I check the public feed the nugets were last updated 5 hours ago (after a manual build I ran with the master branch before Zeeshan's PR). @safern had told me yesterday that sometimes it gets some time (even over an hour) for the Feed to actually get updated, so maybe this is also something that other people relying on the public feed should be aware of.

Copy link
Member Author

@antoniovs1029 antoniovs1029 Oct 29, 2019

Choose a reason for hiding this comment

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

About that last point, it actually seems that the nugets were correctly published to the feed, but the feed decides (for whatever reason) to select previous published nugets and show them directly in the feed instead of the latest ones. If I go to the 'versions' tab of a specific nugget, I actually can see that it got updated on the last builds, but it is selecting to show the one built 8 hours ago:
https://dev.azure.com/dnceng/public/_packaging?_a=package&feed=MachineLearning&package=Microsoft.Extensions.ML&protocolType=NuGet&version=1.4.0-preview3-28229-9&view=versions

I am guessing this is, for some reason, the expected behavior, and that I was wrong when saying that the Feed takes time to reflect the changes... it just doesn't display the latest version on the feed.

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 only have a single feed - the public feed. Having 2 feeds will only increase confusion, storage, and increase the chance of failure.

@antoniovs1029 antoniovs1029 merged commit 1a5a8f4 into dotnet:master Oct 29, 2019
@@ -257,6 +256,14 @@ phases:
msbuildVersion: 15.0
continueOnError: true

- task: NuGetCommand@2
Copy link
Member

Choose a reason for hiding this comment

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

As I mentioned here, I don't think it is worth keeping this.

frank-dong-ms-zz pushed a commit to frank-dong-ms-zz/machinelearning that referenced this pull request Nov 4, 2019
* Publish nugets to azure devops public feed
frank-dong-ms-zz pushed a commit to frank-dong-ms-zz/machinelearning that referenced this pull request Nov 4, 2019
* Publish nugets to azure devops public feed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NuGet packages not being published to public feed
4 participants