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 the broken template scanning pipeline job #7724

Merged
merged 5 commits into from
Mar 18, 2024

Conversation

baronfel
Copy link
Member

Problem

The pipelines for the template cache scanner have been broken for some time now due to an invariant changing - the tools never expected that a template would just...be completely removed.

Solution

We allow this case and add it to the set of removed templates. It can always be re-added back once it's back on NuGet.org.

@baronfel baronfel requested a review from a team as a code owner March 15, 2024 20:16
@@ -0,0 +1,24 @@
{
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 added this file to make f5 debugging of the tool as it is run in the pipeline quite simple to do.

@@ -11,6 +11,7 @@ namespace Microsoft.TemplateSearch.Common
/// <summary>
/// Template package searchable data.
/// </summary>
[System.Diagnostics.DebuggerDisplay("{Name}@{Version}")]
Copy link
Member Author

Choose a reason for hiding this comment

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

This workflow deals with large lists of these items, having a debugger visualization is very handy.

@@ -6,6 +6,7 @@

namespace Microsoft.TemplateSearch.TemplateDiscovery.PackChecking
{
[System.Diagnostics.DebuggerDisplay("{Name}@{Version} - {Reason}")]
Copy link
Member Author

Choose a reason for hiding this comment

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

This workflow deals with large lists of these items, having a debugger visualization is very handy.

throw new Exception($"temp storage path for NuGet packages already exists: {_packageTempPath}");
}
else
if (!Directory.Exists(_packageTempPath))
Copy link
Member Author

Choose a reason for hiding this comment

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

These two changes allow us to re-use the already-downloaded nupkgs when re-running the application. This really cuts down on cycle time.

Copy link
Member

@MiYanni MiYanni left a comment

Choose a reason for hiding this comment

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

:shipit:

@baronfel
Copy link
Member Author

I'm doing a quick compare between my local version of the search cache and the version available at the search cache url - will report back if the diffs seem sensible

@baronfel baronfel marked this pull request as draft March 15, 2024 21:47
@baronfel
Copy link
Member Author

Converted this to a draft because there were a lot of removals in the new cache generated locally, and I'm not confident in the results quite yet.

@baronfel baronfel marked this pull request as ready for review March 16, 2024 21:18
@baronfel
Copy link
Member Author

Ok, I took the time to do a detailed diff and now I'm happy with the results. There's some shuffling of what templates are/are not in the cache file, but these is just a side-effect of the way we are querying the overall NuGet API - because we're stuck at the first 4kish responses due to the API we're using and there's no consistent package ordering, we're going to vary somewhat on the packages in the overall listing.

@@ -12,12 +14,14 @@ internal partial class TemplateSearchCache

internal TemplateSearchCache(IReadOnlyList<TemplatePackageSearchData> data)
{
TemplatePackages = data;
// when creating from freshly-generated data, order the results for clarity
Copy link
Member Author

Choose a reason for hiding this comment

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

This change should make it easier for us to do side-by-side compares in the future. When I did this, it made it very easy to see that most diffs were like this:

image

meaning descriptions, versions, download counts that had changed over time. The other big source of diffs was additions/removals of entire packages from the cache file, which is inevitable until we move to searching the entire NuGet feed.

@baronfel
Copy link
Member Author

Two runs are failing, but I can't see the results - are they not reported in AzDO?

@nagilson
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@baronfel
Copy link
Member Author

Ok, a rerun fixed this. Since it's been approved and I'm confident it in, going to merge it now. We should check the pipeline runs in an hour or so and see if they green up.

@baronfel baronfel merged commit c5efbde into main Mar 18, 2024
10 checks passed
@baronfel baronfel deleted the reenable-template-cache-job branch March 18, 2024 20:32
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.

5 participants