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

samples(storage transfer): add samples and test cases for storage transfer services #2857

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

mahendra-google
Copy link

Adding samples and tests cases for following storage transfer services:

  • 1. storagetransfer_quickstart
  • 2. storagetransfer_transfer_to_nearline
  • 3. storagetransfer_get_latest_transfer_operation
  • 4. storagetransfer_manifest_request
  • 5. storagetransfer_transfer_from_posix
  • 6. storagetransfer_download_to_posix
  • 7. storagetransfer_transfer_posix_to_posix
  • 8. storagetransfer_create_event_driven_gcs_transfer

…utput message in sample output and related small changes.
…sfer to nearline and check latest transfer operation (#1)
…ole , rewording , removing some unnecessary symbols, few test checks removed related to posix file transfer.
@mahendra-google mahendra-google requested review from a team as code owners December 4, 2024 10:22
Copy link

snippet-bot bot commented Dec 4, 2024

Here is the summary of changes.

You are about to add 7 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@product-auto-label product-auto-label bot added api: storagetransfer Issues related to the Storage Transfer Service API. samples Issues that are directly related to samples. labels Dec 4, 2024
@jskeet jskeet assigned amanda-tarafa and unassigned jskeet Dec 4, 2024
@amanda-tarafa amanda-tarafa added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 11, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 11, 2024
Copy link
Member

@amanda-tarafa amanda-tarafa left a 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" />
Copy link
Member

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?

Copy link
Author

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.

@@ -25,7 +25,6 @@ public class QuickstartTest : IDisposable
{
private readonly StorageFixture _fixture;
private string _transferJobName;

Copy link
Member

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 },
Copy link
Member

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?

Copy link
Author

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();
Copy link
Member

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?

Comment on lines 38 to 40
byte[] byteArray = Encoding.UTF8.GetBytes("flower.jpeg");
MemoryStream stream = new MemoryStream(byteArray);
storage.UploadObject(_fixture.BucketNameSource, _fixture.ManifestObjectName, "application/octet-stream", stream);
Copy link
Member

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.

Copy link
Member

@amanda-tarafa amanda-tarafa left a comment

Choose a reason for hiding this comment

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

mahendra-google and others added 5 commits December 19, 2024 14:49
…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>
@amanda-tarafa
Copy link
Member

@mahendra, there are still conflicts in your branch. Let me know when it's ready for review again, thanks!

@mahendra
Copy link

@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.

@amanda-tarafa
Copy link
Member

@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.

@amanda-tarafa
Copy link
Member

amanda-tarafa commented Dec 20, 2024

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.

@amanda-tarafa amanda-tarafa added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 20, 2024
@kokoro-team kokoro-team removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Dec 20, 2024
@amanda-tarafa
Copy link
Member

Tests are green, thanks. I'll re-review as soon as possible, but likely to be in January.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storagetransfer Issues related to the Storage Transfer Service API. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants