-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
samples(storage transfer): add samples and test cases for storage transfer services #2857
base: main
Are you sure you want to change the base?
samples(storage transfer): add samples and test cases for storage transfer services #2857
Conversation
…transfer_transfer_to_nearline.
…b.com/mahendra-google/dotnet-docs-samples into missing_.net_transfer_service_samples
…storagetransfer_transfer_to_nearline.
…or storagetransfer_transfer_to_nearline.
…b.com/mahendra-google/dotnet-docs-samples into missing_.net_transfer_service_samples
…utput message in sample output and related small changes.
…andard output of test log summary.
…test transfer operation.
…sfer to nearline and check latest transfer operation (#1)
…ket using manifest file (#2)
… folder in posix file system (#5)
…ole , rewording , removing some unnecessary symbols, few test checks removed related to posix file transfer.
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.
I only reviewed this partially but there are plenty of change requests that will apply to all samples and tests in this PR.
Splitting this PR in several, each container 2 samples and their tests, will make things easier to review.
@@ -6,5 +6,6 @@ | |||
|
|||
<ItemGroup> | |||
<PackageReference Include="Google.Cloud.StorageTransfer.V1" Version="2.7.0" /> | |||
<PackageReference Include="xunit.abstractions" Version="2.0.3" /> |
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 would you need this on the samples project?
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.
I forgot to remove that I added for the output helper. I will remove this.
storagetransfer/api/StorageTransfer.Samples/QuickstartSample.cs
Outdated
Show resolved
Hide resolved
storagetransfer/api/StorageTransfer.Samples.Tests/QuickstartTest.cs
Outdated
Show resolved
Hide resolved
@@ -25,7 +25,6 @@ public class QuickstartTest : IDisposable | |||
{ | |||
private readonly StorageFixture _fixture; | |||
private string _transferJobName; | |||
|
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.
Revert
@@ -36,8 +35,8 @@ public TransferJob Quickstart( | |||
ProjectId = projectId, | |||
TransferSpec = new TransferSpec | |||
{ | |||
GcsDataSink = new GcsData { BucketName = sourceBucket }, | |||
GcsDataSource = new GcsData { BucketName = sinkBucket } | |||
GcsDataSink = new GcsData { BucketName = sinkBucket }, |
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 surprising that the test needed no changes. Are we testing for the right thing?
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.
Yes its just checking transfer job created or not .
public void TransferUsingManifest() | ||
{ | ||
TransferUsingManifestSample transferUsingManifestSample = new TransferUsingManifestSample(); | ||
var storage = StorageClient.Create(); |
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.
Don't you have one in the fixture?
storagetransfer/api/StorageTransfer.Samples.Tests/TransferUsingManifestTest.cs
Show resolved
Hide resolved
byte[] byteArray = Encoding.UTF8.GetBytes("flower.jpeg"); | ||
MemoryStream stream = new MemoryStream(byteArray); | ||
storage.UploadObject(_fixture.BucketNameSource, _fixture.ManifestObjectName, "application/octet-stream", stream); |
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.
This seems like the kind of thing that you would add to the fixture, i.e. create s bucket with some data in it that you can use to test transfer jobs.
But, the quickstart was not actually transferring anything. We really don't test the service in samples, we test the samples themselves, so creating and running a transfer job from and empty source seems enough for that.
storagetransfer/api/StorageTransfer.Samples.Tests/StorageTransfer.Samples.Tests.csproj
Show resolved
Hide resolved
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.
Thanks @JesseLovelace
storagetransfer/api/StorageTransfer.Samples.Tests/StorageTransfer.Samples.Tests.csproj
Show resolved
Hide resolved
storagetransfer/api/StorageTransfer.Samples.Tests/TransferUsingManifestTest.cs
Show resolved
Hide resolved
…estSample.cs Co-authored-by: Amanda Tarafa Mas <14878252+amanda-tarafa@users.noreply.github.com>
…gManifestTest.cs Co-authored-by: Amanda Tarafa Mas <14878252+amanda-tarafa@users.noreply.github.com>
…ithub.com/mahendra-google/dotnet-docs-samples into event_driven_gcs_transfer_service_samples
@mahendra, there are still conflicts in your branch. Let me know when it's ready for review again, thanks! |
@amanda-tarafa - I think you meant @mahendra-google and not me. |
Yes, you are right, sorry for the noise. |
Just as a heads up, it's unlikely that I can get to this before January. I'm running the tests though so we at least know whether things are working or not. |
Tests are green, thanks. I'll re-review as soon as possible, but likely to be in January. |
Adding samples and tests cases for following storage transfer services: