Publish artifacts and symbols one file at a time #7086
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,21 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Text.RegularExpressions; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.Build.Framework; | ||
| using Microsoft.DotNet.Build.CloudTestTasks; | ||
| using Microsoft.WindowsAzure.Storage; | ||
| using Newtonsoft.Json; | ||
| using Newtonsoft.Json.Linq; | ||
| using Newtonsoft.Json.Serialization; | ||
| using NuGet.Packaging.Core; | ||
| using Sleet; | ||
| using MSBuild = Microsoft.Build.Utilities; | ||
|
|
||
| namespace Microsoft.DotNet.Build.Tasks.Feed | ||
|
|
@@ -124,26 +122,24 @@ public async Task<bool> PushItemsToFeedAsync( | |
| return !Log.HasLoggedErrors; | ||
| } | ||
|
|
||
| public async Task PublishToFlatContainerAsync(IEnumerable<ITaskItem> taskItems, int maxClients, | ||
| PushOptions pushOptions) | ||
| { | ||
| if (taskItems.Any()) | ||
| { | ||
| using (var clientThrottle = new SemaphoreSlim(maxClients, maxClients)) | ||
| { | ||
| await System.Threading.Tasks.Task.WhenAll(taskItems.Select( | ||
| item => { return UploadAssetAsync(item, pushOptions, clientThrottle); } | ||
| )); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public async Task UploadAssetAsync( | ||
epananth marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ITaskItem item, | ||
| PushOptions options, | ||
| SemaphoreSlim clientThrottle = null) | ||
| { | ||
| string relativeBlobPath = item.GetMetadata("RelativeBlobPath"); | ||
|
|
||
|
|
@@ -154,58 +150,72 @@ public async Task UploadAssetAsync( | |
| relativeBlobPath = $"{recursiveDir}{fileName}"; | ||
michellemcdaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| if (!string.IsNullOrEmpty(relativeBlobPath)) | ||
| { | ||
| relativeBlobPath = $"{RelativePath}{relativeBlobPath}".Replace("\\", "/"); | ||
|
|
||
| if (relativeBlobPath.StartsWith("//")) | ||
| { | ||
| Log.LogError( | ||
| $"Item '{item.ItemSpec}' RelativeBlobPath contains virtual directory " + | ||
| $"without name (double forward slash): '{relativeBlobPath}'"); | ||
| return; | ||
| } | ||
|
|
||
| Log.LogMessage($"Uploading {relativeBlobPath}"); | ||
|
|
||
| if (clientThrottle != null) | ||
| { | ||
| await clientThrottle.WaitAsync(); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| AzureStorageUtils blobUtils = new AzureStorageUtils(AccountName, AccountKey, ContainerName); | ||
|
|
||
| if (!options.AllowOverwrite && await blobUtils.CheckIfBlobExistsAsync(relativeBlobPath)) | ||
| { | ||
| if (options.PassIfExistingItemIdentical) | ||
| { | ||
| if (!await blobUtils.IsFileIdenticalToBlobAsync(item.ItemSpec, relativeBlobPath)) | ||
| { | ||
| Log.LogError( | ||
| $"Item '{item}' already exists with different contents " + | ||
| $"at '{relativeBlobPath}'"); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| Log.LogError($"Item '{item}' already exists at '{relativeBlobPath}'"); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| using (FileStream stream = | ||
| new FileStream(item.ItemSpec, FileMode.Open, FileAccess.Read, FileShare.Read)) | ||
| { | ||
| Log.LogMessage($"Uploading {item} to {relativeBlobPath}."); | ||
| await blobUtils.UploadBlockBlobAsync(item.ItemSpec, relativeBlobPath, stream); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception exc) | ||
| { | ||
| Log.LogError( | ||
| $"Unable to upload to {relativeBlobPath} in Azure Storage account {AccountName}/{ContainerName} due to {exc}."); | ||
| throw; | ||
| } | ||
| finally | ||
| { | ||
| if (clientThrottle != null) | ||
| { | ||
| clientThrottle.Release(); | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| Log.LogError($"Relative blob path is empty."); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -332,7 +342,7 @@ private ISleetFileSystem GetAzureFileSystem(LocalCache fileCache) | |
| } | ||
| } | ||
|
|
||
| public async Task<bool> PushAsync(IEnumerable<string> items, PushOptions options) | ||
|
There was a problem hiding this comment. It looks like this method is basically wrapped by https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Build.Tasks.Feed/src/BlobFeedAction.cs#L103, Is there a reason to make this public instead of using that? There was a problem hiding this comment. I am calling PushAsync in PublishArtifactsInManifestBase, before it was called only in BlobFeedAction There was a problem hiding this comment. Yes, what I'm saying is that there's another method (the one I link in my comment) that is already public and calls into this private method that you're making public. My question is whether that is intentional, or just that you didn't see the other one. There was a problem hiding this comment. Yes other method which calls PushAsync which is PushToFeedAsync, does a sanity check to see if there are any duplicated in the ItemsList. Since I'm only pushing one item at a time. That part is not required. So I called PushAsync directly There was a problem hiding this comment. So, looking at arcade/src/Microsoft.DotNet.Build.Tasks.Feed/src/BlobFeedAction.cs Lines 103 to 125 in 4535393 Alternatively, you could add parameter validation and error handling, but then you'd end up with pretty much the same method as the one I'm saying to use. |
||
| { | ||
| LocalSettings settings = GetSettings(); | ||
| SleetLogger log = new SleetLogger(Log, NuGet.Common.LogLevel.Verbose); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the extra slash at the end necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I copied the paths we already had in the yaml. But I checked without the / or with the / it will work.