-
Notifications
You must be signed in to change notification settings - Fork 344
AzCopyCore updates #2171
AzCopyCore updates #2171
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,9 @@ | |
<PropertyGroup> | ||
<OutputType>Exe</OutputType> | ||
<TargetFramework>netcoreapp2.1</TargetFramework> | ||
<!-- | ||
<RuntimeFrameworkVersion>2.1.0-preview2-26209-04</RuntimeFrameworkVersion> | ||
--> | ||
<CoreFxPackageVersion>4.5.0-preview2-26213-06</CoreFxPackageVersion> | ||
<CoreFxLabPackageVersion>0.1.0-preview2-180213-4</CoreFxLabPackageVersion> | ||
</PropertyGroup> | ||
|
@@ -19,18 +21,19 @@ | |
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Microsoft.DotNet.ILCompiler" Version="1.0.0-alpha-26223-02" /> | ||
<PackageReference Include="System.Memory" Version="$(CoreFxPackageVersion)" /> | ||
<PackageReference Include="System.Threading.Tasks.Extensions" Version="$(CoreFxPackageVersion)" /> | ||
<PackageReference Include="Microsoft.DotNet.ILCompiler" Version="1.0.0-alpha-26320-02" /> | ||
<PackageReference Include="System.Buffers.ReaderWriter" Version="0.1.0-preview2-180320-2" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Move this down to the other ItemGroup with the rest of the corefxlab packages There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do |
||
<PackageReference Include="System.Memory" Version="4.5.0-preview3-26319-04" /> | ||
<PackageReference Include="System.Threading.Tasks.Extensions" Version="4.5.0-preview3-26319-04" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update and use the CoreFxPackageVersion property to make it easier to update in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I said above, I like how the sample shows VS experience that our customers will get. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if someone would go and manually update each individual referenced packages using VS/NuGet package manager. Once they are added (which can be done by using VS/NuGet package manager), it would make sense to create a property to update the versions, no? |
||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="System.Azure.Experimental" Version="$(CoreFxLabPackageVersion)" /> | ||
<PackageReference Include="System.Buffers.Experimental" Version="$(CoreFxLabPackageVersion)" /> | ||
<PackageReference Include="System.Text.Http" Version="$(CoreFxLabPackageVersion)" /> | ||
<PackageReference Include="System.Text.Http.Parser" Version="$(CoreFxLabPackageVersion)" /> | ||
<PackageReference Include="System.Text.Utf8String" Version="$(CoreFxLabPackageVersion)" /> | ||
<PackageReference Include="System.IO.Pipelines" Version="$(CoreFxLabPackageVersion)" /> | ||
<PackageReference Include="System.Azure.Experimental" Version="0.1.0-preview2-180320-2" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update and use the CoreFxLabPackageVersion property to make it easier to update in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, but there is a trade off here. I think it's good that we have some samples that validate how our customers are using and demonstrate how they should be using our packages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the trade off here. We have all these package version numbers here, and they point to the same version. We have a property in this csproj called CoreFxLabPackageVersion. I was just suggesting we use that so that it becomes a single point of edit in the future, if we ever want to upgrade them again. Note, this is still isolated from the rest of the repo and would still demonstrate the customer's use of our packages. |
||
<PackageReference Include="System.Buffers.Experimental" Version="0.1.0-preview2-180320-2" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can these be project references instead of package references? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above: I think samples should show customer experience that does not require cloning the whole repo. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. In that case, package references make sense. Thanks. |
||
<PackageReference Include="System.Text.Http" Version="0.1.0-preview2-180320-2" /> | ||
<PackageReference Include="System.Text.Http.Parser" Version="0.1.0-preview2-180320-2" /> | ||
<PackageReference Include="System.Text.Utf8String" Version="0.1.0-preview2-180320-2" /> | ||
<PackageReference Include="System.IO.Pipelines" Version="4.5.0-preview3-26319-04" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Move this up to the other ItemGroup with the rest of the CoreFX packages There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do |
||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Buffers; | ||
using System.IO.Pipelines; | ||
using System.Text.Http.Parser; | ||
using System.Threading.Tasks; | ||
|
||
// SocketClient is an experimental low-allocating/low-copy HTTP client API | ||
// TODO (pri 2): need to support cancellations | ||
namespace System.Net.Experimental | ||
{ | ||
public readonly struct PipeHttpClient | ||
{ | ||
static readonly HttpParser s_headersParser = new HttpParser(); | ||
|
||
readonly IDuplexPipe _pipe; | ||
|
||
public PipeHttpClient(IDuplexPipe pipe) | ||
{ | ||
_pipe = pipe; | ||
} | ||
|
||
public async ValueTask<TResponse> SendRequest<TRequest, TResponse>(TRequest request) | ||
where TRequest : IPipeWritable | ||
where TResponse : IHttpResponseHandler, new() | ||
{ | ||
await request.WriteAsync(_pipe.Output).ConfigureAwait(false); | ||
|
||
PipeReader reader = _pipe.Input; | ||
TResponse response = await ParseResponseAsync<TResponse>(reader).ConfigureAwait(false); | ||
await response.OnBody(reader); | ||
return response; | ||
} | ||
|
||
static async ValueTask<T> ParseResponseAsync<T>(PipeReader reader) | ||
where T : IHttpResponseHandler, new() | ||
{ | ||
var handler = new T(); | ||
while (true) | ||
{ | ||
ReadResult result = await reader.ReadAsync(); | ||
ReadOnlySequence<byte> buffer = result.Buffer; | ||
// TODO (pri 2): this should not be static, or ParseHeaders should be static | ||
if (HttpParser.ParseResponseLine(ref handler, ref buffer, out int rlConsumed)) | ||
{ | ||
reader.AdvanceTo(buffer.GetPosition(rlConsumed)); | ||
break; | ||
} | ||
reader.AdvanceTo(buffer.Start, buffer.End); | ||
} | ||
|
||
while (true) | ||
{ | ||
ReadResult result = await reader.ReadAsync(); | ||
ReadOnlySequence<byte> buffer = result.Buffer; | ||
if (s_headersParser.ParseHeaders(ref handler, buffer, out int hdConsumed)) | ||
{ | ||
reader.AdvanceTo(buffer.GetPosition(hdConsumed)); | ||
break; | ||
} | ||
reader.AdvanceTo(buffer.Start, buffer.End); | ||
} | ||
|
||
await handler.OnBody(reader); | ||
return handler; | ||
} | ||
|
||
public bool IsConnected => _pipe != null; | ||
} | ||
|
||
public interface IHttpResponseHandler : IHttpHeadersHandler, IHttpResponseLineHandler | ||
{ | ||
ValueTask OnBody(PipeReader body); | ||
} | ||
} | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,22 +28,19 @@ static void PrintUsage() | |
static void Main(string[] args) | ||
{ | ||
Log.Listeners.Add(new ConsoleTraceListener()); | ||
Log.Switch.Level = SourceLevels.Error; | ||
|
||
long before = GC.GetAllocatedBytesForCurrentThread(); | ||
Log.Switch.Level = SourceLevels.Information; | ||
|
||
var options = new CommandLine(args); | ||
ReadOnlySpan<char> source = options.GetSpan("/Source:"); | ||
ReadOnlySpan<char> destination = options.GetSpan("/Dest:"); | ||
|
||
long before = GC.GetAllocatedBytesForCurrentThread(); | ||
var sw = Stopwatch.StartNew(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: use of var There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will fix |
||
|
||
// transfer from file system to storage | ||
if (destination.StartsWith("http://")) | ||
{ | ||
var sw = new Stopwatch(); | ||
sw.Start(); | ||
TransferDirectoryToStorage(source, destination, options); | ||
sw.Stop(); | ||
Console.WriteLine("Elapsed time: " + sw.ElapsedMilliseconds + " ms"); | ||
} | ||
|
||
// transfer from storage to file system | ||
|
@@ -54,8 +51,14 @@ static void Main(string[] args) | |
|
||
else { PrintUsage(); } | ||
|
||
sw.Stop(); | ||
long after = GC.GetAllocatedBytesForCurrentThread(); | ||
Console.WriteLine($"GC Allocations: {after - before} bytes"); | ||
|
||
if (Log != null && Log.Switch.ShouldTrace(TraceEventType.Information)) | ||
{ | ||
Log.TraceInformation("Elapsed time: " + sw.ElapsedMilliseconds + " ms"); | ||
Log.TraceInformation($"GC Allocations: {after - before} bytes"); | ||
} | ||
|
||
if (Debugger.IsAttached) | ||
{ | ||
|
@@ -76,7 +79,9 @@ static void TransferDirectoryToStorage(ReadOnlySpan<char> localDirectory, ReadOn | |
byte[] keyBytes; | ||
if (options.Contains("/DestKey:")) | ||
{ | ||
keyBytes = options["/DestKey:"].ComputeKeyBytes(); | ||
ReadOnlySpan<char> key = options["/DestKey:"]; | ||
var str = key.ToString(); | ||
keyBytes = key.ComputeKeyBytes(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this change necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. leftover from debugging. I removed now. thanks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
else if (options.Contains("/@:")) | ||
{ | ||
|
@@ -96,9 +101,9 @@ static void TransferDirectoryToStorage(ReadOnlySpan<char> localDirectory, ReadOn | |
ReadOnlySpan<char> host = storageFullPath.Slice(0, pathStart); | ||
ReadOnlySpan<char> path = storageFullPath.Slice(pathStart + 1); | ||
|
||
using (var client = new StorageClient(keyBytes, account, host)) | ||
using (var client = new StorageClient(keyBytes, account, host, 80, Log)) | ||
{ | ||
client.Log = Log; | ||
//var files = new Files(directoryPath); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clean up commented out code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do. |
||
foreach (string filepath in Directory.EnumerateFiles(directoryPath)) | ||
{ | ||
// TODO: Use Path.Join when it becomes available | ||
|
@@ -115,7 +120,7 @@ static void TransferDirectoryToStorage(ReadOnlySpan<char> localDirectory, ReadOn | |
} | ||
} | ||
|
||
private static bool TryGetKey(string filename, out ReadOnlySpan<char> line) | ||
static bool TryGetKey(string filename, out ReadOnlySpan<char> line) | ||
{ | ||
if (!File.Exists(filename)) | ||
{ | ||
|
@@ -129,7 +134,7 @@ private static bool TryGetKey(string filename, out ReadOnlySpan<char> line) | |
string firstLine; | ||
if ((firstLine = streamReader.ReadLine()) != null) | ||
{ | ||
line = firstLine.AsReadOnlySpan(); | ||
line = firstLine.AsSpan(); | ||
} | ||
else | ||
{ | ||
|
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.
Why is this commented out? This should be the same as https://github.com/dotnet/corefxlab/blob/master/SharedRuntimeVersion.txt (or import the common.props file).
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.
It's @terrajobst's fault. :-) I removed it now.