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

First ResolveUri in NuGet.Services.Storage #10129

Merged
merged 1 commit into from
Aug 15, 2024
Merged

First ResolveUri in NuGet.Services.Storage #10129

merged 1 commit into from
Aug 15, 2024

Conversation

joelverhagen
Copy link
Member

@joelverhagen joelverhagen commented Aug 14, 2024

This fixes the following jobs on the new storage SDK.

ResolveUri was not working since the container URL lost it's trailing /, leading to new Uri(base, blob) behavior changing. The cursor URL was resolving to the root of the blob storage account, not in the storage container.

GitHubVulnerabilities2Db and GitHubVulnernabilities2v3 were impacted.

Summary of changes:

  • Fix Storage.ResolveUri to be tolerant of trailing and leading /. This fixes the GitHubVulnerabilities job cursors.
  • Fix DateTimeOffset.MinValue comparison.
    • On a UTC machine DateTimeOffset.MinValue == DateTime.MinValue (our VMs). Locally this is not true. This makes local debugging harder. I think this was left over from a DateTime to DateTimeOffset refactoring that was uncaught cause it works on VM.

@joelverhagen joelverhagen requested a review from a team as a code owner August 14, 2024 22:13
@joelverhagen joelverhagen merged commit 978a209 into dev Aug 15, 2024
2 checks passed
@joelverhagen joelverhagen deleted the jver-fix-vuln branch August 15, 2024 03:30
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