(feat) Storage Transfer Service: add gapic samples and refactor test suite #6120

JesseLovelace · 2021-10-01T20:36:08Z

This adds the gapic samples for transfer service, replacing the old apiary samples as the "main samples", while marking the apiary samples as apiary. The new gapic samples will be on the main documentation page, while the apiary samples will be in a migration guide.

This is also refactors the test suite. Previously, the tests relied on hard-coded buckets that have the correct permissions set up. This is potentially flaky, and also makes checking out the samples and running the tests difficult. Now, buckets are simply spun up, given the correct permissions, and then deleted at the end of the tests.

Do not merge label: this shouldn't be merged until all artifacts related to the migration to gapic (i.e.: the python samples, the migration guide, the quickstart guides, the CLs publishing all these), are also ready to go

snippet-bot · 2021-10-01T20:36:16Z

Here is the summary of possible violations 😱

There is a possible violation for not having product prefix.

storage-transfer/src/test/java/com/google/cloud/storage/storagetransfer/samples/test/util/TransferJobUtils.java:17, tag all

The end of the violation section. All the stuff below is FYI purposes only.

You are about to delete the following frozen region tags.

storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/CheckLatestTransferOperation.java:19, tag storagetransfer_get_latest_transfer_operation(usage: page 1).
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/CreateTransferClient.java:19, tag storagetransfer_create_client(usage: page 1).
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/RetryHttpInitializerWrapper.java:19, tag storagetransfer_create_retry_handler(usage: page 1).
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferFromAws.java:19, tag storagetransfer_transfer_from_aws(usage: page 1).
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferToNearline.java:19, tag storagetransfer_transfer_to_nearline(usage: page 1).

Here is the summary of changes.

You are about to add 11 region tags.

storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/CheckLatestTransferOperation.java:19, tag storagetransfer_get_latest_transfer_operation
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/QuickstartSample.java:19, tag storagetransfer_quickstart
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferFromAws.java:19, tag storagetransfer_transfer_from_aws
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferToNearline.java:19, tag storagetransfer_transfer_to_nearline
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/apiary/CheckLatestTransferOperationApiary.java:19, tag storagetransfer_get_latest_transfer_operation_apiary
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/apiary/CheckTransferJobApiary.java:19, tag storagetransfer_get_transfer_job
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/apiary/CreateTransferClient.java:19, tag storagetransfer_create_client
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/apiary/RetryHttpInitializerWrapper.java:19, tag storagetransfer_create_retry_handler
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/apiary/TransferFromAwsApiary.java:19, tag storagetransfer_transfer_from_aws_apiary
storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/apiary/TransferToNearlineApiary.java:19, tag storagetransfer_transfer_to_nearline_apiary
storage-transfer/src/test/java/com/google/cloud/storage/storagetransfer/samples/test/util/TransferJobUtils.java:17, tag all

You are about to delete 7 region tags.

storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/CheckLatestTransferOperation.java:19, tag storagetransfer_get_latest_transfer_operation
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/CheckTransferJob.java:19, tag storagetransfer_get_transfer_job
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/CreateTransferClient.java:19, tag storagetransfer_create_client
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/RetryHttpInitializerWrapper.java:19, tag storagetransfer_create_retry_handler
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferFromAws.java:19, tag storagetransfer_transfer_from_aws
storage/storage-transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferToNearline.java:19, tag storagetransfer_transfer_to_nearline
storage/storage-transfer/src/test/java/com/google/cloud/storage/storagetransfer/samples/test/util/TransferJobUtils.java:17, tag all

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

frankyn

LGTM, just have one question on directory structure, the code changes to the samples make this a very appealing migration for existing developers because the delta isn't as heavy.
It still does have a list of data models to worry about but the main thing that's awesome is the removal of auth init.

frankyn · 2021-10-01T22:40:24Z

...transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferFromAws.java

    // Set up the transfer job
    TransferJob transferJob =
-        new TransferJob()
+        TransferJob.newBuilder()


this is so nice and a lot smaller delta

frankyn · 2021-10-01T22:43:34Z

storage/storage-transfer/pom.xml

        </exclusion>
      </exclusions>
    </dependency>
+    <dependency>


One question on directory structure, wdyt about moving these files one directory up outside of the storage/ directory because it's a different API?

I agree, since it's structured that way under googleapis, i think it makes sense to mirror that structure here

frankyn · 2021-10-17T19:47:15Z

...ransfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/QuickstartSample.java

+    System.out.println("Created transfer job between two GCS buckets:");
+    System.out.println(response.toString());
+  }
+}


Way more reasonable, thanks for pushing on this Jesse!

frankyn · 2021-10-17T19:49:38Z

...ransfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/QuickstartSample.java

+import com.google.storagetransfer.v1.proto.TransferTypes.TransferSpec;
+
+public class QuickstartSample {
+  /** Quickstart sample using transfer service to transfer from one GCS bucket to another. */


Suggestion ...immediately transfer from one GCS bucket to another. because datetime is not supplied.

Actually, running immediately isn't what happens. Based on the jobs created from my local runs, what actually happens is that the job is created, but does not run, and does not have any runs scheduled. To run it, you would need to do so in the console or call runTransferJob().

Should I update the quickstart to also call runTransferJob()?

thanks for clarifying, I missed that, documentation calls out that if StartTime isn't specified it will auto-start, but it may require a scheduled date to run. I think calling runTransferJob() after seems like the least complex to add after creating a job. I feel like scheduling is an advance setting.

@danielbankhead wdyt?

Hey, just now seeing this - I think either is fine, but runTransferJob might be a bit more complex for the customer (a user would have to know to disable to job). While more verbose, I think scheduling would make more sense (although it is a somewhat of a misnomer if we want to run this one time). For this reason, I think a one-time helper function would be nice to have across the GAPIC clients.

What do you mean by "a user would have to know to disable to job"?

Jobs that are created and then run this way, only run once. There's no need to disable it

You're correct that there isn't a reason to explicitly disable it; I'm thinking someone new to the product may be concerned because 'Frequency' isn't set and the documentation mentions 'Schedule one-time transfer operations or recurring transfer operations', making it a bit ambiguous if this transfer job will run again in the future.

Setting the schedule to the same date sets the Frequency to 'Once' in the UI - making it clearer to the user that this won't run again in the future.

I think this would be valuable to capture as feedback and share upstream

frankyn · 2021-11-01T20:59:43Z

...ransfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/QuickstartSample.java

+            .setStatus(TransferJob.Status.ENABLED)
+            .build();
+
+    StorageTransferServiceClient storageTransfer = StorageTransferServiceClient.create();


Could you move client creation before transferJob?

frankyn · 2021-11-01T21:01:00Z

...ransfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/QuickstartSample.java

+            CreateTransferJobRequest.newBuilder().setTransferJob(transferJob).build());
+
+    storageTransfer
+        .runTransferJobCallable()


Is this the only way to start the job? Wondering if there's an equivalent method such as storageTransfer.createTransferJob() -> storageTransfer.runTransferJob()?

The only other way would be runTransferJobAsync(). There is no runTransferJob().

Thanks for checking! Disregard this comment.

Would look like

storageTransfer.runTransferJobAsync( TransferProto.RunTransferJobRequest.newBuilder() .setProjectId(projectId) .setJobName(response.getName()) .build()) .get();

Well it's nice that we have async, but let's leave that for our readers to find.

frankyn

Few Nits

frankyn · 2021-11-02T18:04:17Z

...transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferFromAws.java

@@ -0,0 +1,129 @@
+/*
+ * Copyright 2020 Google Inc.


frankyn · 2021-11-02T18:05:27Z

...java/com/google/cloud/storage/storagetransfer/samples/test/ITStoragetransferSamplesTest.java

@@ -0,0 +1,388 @@
+package com.google.cloud.storage.storagetransfer.samples.test;


Missing copyright

frankyn · 2021-11-02T18:06:01Z

...main/java/com/google/cloud/storage/storagetransfer/samples/CheckLatestTransferOperation.java

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2020 Google Inc.


frankyn · 2021-11-02T18:07:12Z

...transfer/src/main/java/com/google/cloud/storage/storagetransfer/samples/TransferFromAws.java

+            .build();
+
+    // Create a Transfer Service client
+    StorageTransferServiceClient storageTransfer = StorageTransferServiceClient.create();


Do you want to move client creation before model construction?

I can go either way on it. My idea was just having client creation right before the client is actually used, but I can move it up if you prefer

I was thinking consistency, but I think your right in this case. You're only creating a client when you actually need it.

add gapic samples and refactor test suite

37cc30c

JesseLovelace added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 1, 2021

JesseLovelace requested a review from frankyn October 1, 2021 20:36

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 1, 2021

product-auto-label bot added the samples Issues that are directly related to samples. label Oct 1, 2021

blunderbuss-gcf bot assigned anguillanneuf Oct 1, 2021

frankyn suggested changes Oct 1, 2021

View reviewed changes

Add quickstart, move to own directory, run formatter

fe05bcd

frankyn suggested changes Oct 25, 2021

View reviewed changes

Add cleanup to tests

f466062

frankyn reviewed Nov 1, 2021

View reviewed changes

JesseLovelace added 3 commits November 2, 2021 11:01

move client creation up in quickstart

c5be104

Merge remote-tracking branch 'origin/main' into stsgapicsamples

344d78d

Fix import

0c1897f

frankyn suggested changes Nov 2, 2021

View reviewed changes

fix copyrights

e6134a3

frankyn approved these changes Nov 2, 2021

View reviewed changes

Merge remote-tracking branch 'origin/main' into stsgapicsamples

f4541ca

JesseLovelace removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 24, 2021

JesseLovelace merged commit 1cdb592 into main Nov 24, 2021

JesseLovelace deleted the stsgapicsamples branch November 24, 2021 20:30

		@@ -0,0 +1,388 @@
		package com.google.cloud.storage.storagetransfer.samples.test;

(feat) Storage Transfer Service: add gapic samples and refactor test suite #6120

(feat) Storage Transfer Service: add gapic samples and refactor test suite #6120

Uh oh!

Conversation

JesseLovelace commented Oct 1, 2021

Uh oh!

snippet-bot bot commented Oct 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankyn Oct 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

snippet-bot bot commented Oct 1, 2021 •

edited

Loading

frankyn Oct 29, 2021 •

edited

Loading