chore: added regional endpoint sample for datastore #1043

shweta345 · 2023-04-10T06:03:28Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

snippet-bot · 2023-04-10T06:03:36Z

Here is the summary of changes.

You are about to add 1 region tag.

samples/snippets/src/main/java/com/example/datastore/RegionalEndpoint.java:20, tag datastore_regional_endpoint

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

samples/snippets/src/main/java/com/example/datastore/QuickstartSample.java

samples/snippets/src/test/java/com/example/datastore/RegionalEndpointIT.java

samples/snippets/src/main/java/com/example/datastore/RegionalEndpoint.java

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

telpirion · 2023-04-12T20:40:54Z

samples/snippets/src/main/java/com/example/datastore/RegionalEndpoint.java

+
+  public Datastore createClient() throws Exception {
+    // Instantiates a client
+    // [START datastore_regional_endpoint]


nit: current best practice is to include import statements and the sample class inside the region tags.

See:
https://googlecloudplatform.github.io/samples-style-guide/#region-tags

telpirion · 2023-04-12T20:45:15Z

samples/snippets/src/test/java/com/example/datastore/RegionalEndpointIT.java

+
+  @After
+  public void tearDown() {
+    System.setOut(null);


issue: Remove this. If you use the SystemsOutRule, you don't need to change or restore what goes pipes to stdout.

telpirion · 2023-04-12T20:47:11Z

samples/snippets/src/test/java/com/example/datastore/RegionalEndpointIT.java

+    // Saves the entity
+    datastoreWithEndpoint.put(task);
+
+    System.out.printf("Saved %s: %s%n", task.getKey().getName(), task.getString("description"));


nit: Remove print statements. Tests are most commonly run in a CI/CD environment where output is written to logs.

telpirion · 2023-04-12T20:51:01Z

samples/snippets/src/test/java/com/example/datastore/RegionalEndpointIT.java

+
+    System.out.printf("Retrieved %s: %s%n", taskKey.getName(), retrieved.getString("description"));
+
+    systemsOutRule.assertContains("Saved sampletask1: Buy milk");


issue: it looks like this test is upserting an entity and then querying the Datastore for the same entity. You don't need to do a string comparison here. Instead just compare the Entity you upsert to the Entity you receive.

lesv

PTAL

Also, if I'm a listed reviewer and I haven't responded in a few days, feel free to ping me.

lesv · 2023-04-26T16:46:20Z

samples/snippets/src/test/java/com/example/datastore/RegionalEndpointIT.java

+    // The kind for the new entity
+    String kind = "Task";
+    // The name/ID for the new entity
+    String name = "sampletask1";


These are created in a common project, and many of these tests can be run at least 4x simultaneously. You should strongly consider using a UUID as name, possibly with a common prefix to tell you which test created, but failed before deleting the object.

Updated to an identifiable ID. Thank you.

Use an identifiable name for the test entity.

lesv · 2023-04-26T23:11:25Z

samples/snippets/src/test/java/com/example/datastore/RegionalEndpointIT.java

+    // The kind for the new entity
+    String kind = "Task";
+    // The name/ID for the new entity
+    String name = "regionalEndpointClient50720906";


Not quite what I had in mind. See https://docs.oracle.com/javase/8/docs/api/java/util/UUID.html

You probably want to create one once and use it in both the testRegionalEndpoint() and deleteTestEntity() methods.

Ah, thank you for explaining. Updated to use UUID. Does a UUID.randomUUID() make sense here?

shweta345 · 2023-05-09T03:51:31Z

@telpirion are there any other changes pending or can this PR be merged?

chore: added regional endpoint sample for datastore

f8f37af

product-auto-label bot added the size: m Pull request size is medium. label Apr 10, 2023

product-auto-label bot added api: datastore Issues related to the googleapis/java-datastore API. samples Issues that are directly related to samples. labels Apr 10, 2023

blunderbuss-gcf bot assigned lesv Apr 10, 2023

chore: added regional endpoint sample for datastore

3c3b655

kolea2 requested changes Apr 10, 2023

View reviewed changes

samples/snippets/src/main/java/com/example/datastore/QuickstartSample.java Outdated Show resolved Hide resolved

samples/snippets/src/test/java/com/example/datastore/RegionalEndpointIT.java Outdated Show resolved Hide resolved

kolea2 requested changes Apr 10, 2023

View reviewed changes

samples/snippets/src/test/java/com/example/datastore/RegionalEndpointIT.java Outdated Show resolved Hide resolved

jlara310 added 2 commits April 11, 2023 16:43

revert changes to QuickstartSample.java

117c621

Refactor test. Move validation operations to the test.

0bcafad

kolea2 reviewed Apr 11, 2023

View reviewed changes

samples/snippets/src/main/java/com/example/datastore/RegionalEndpoint.java Outdated Show resolved Hide resolved

Use correct api endpoint.

e0eda8e

kolea2 added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Apr 12, 2023

remove typo

ec72ff5

kolea2 approved these changes Apr 12, 2023

View reviewed changes

kolea2 added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Apr 12, 2023

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 12, 2023

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 12, 2023

🦉 Updates from OwlBot post-processor

2cdaf5d

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

gcf-owl-bot bot requested review from a team as code owners April 12, 2023 19:32

telpirion suggested changes Apr 12, 2023

View reviewed changes

Refactor test

eeff0ae

lesv suggested changes Apr 26, 2023

View reviewed changes

Use a unique ID for the test entity

dc41325

Use an identifiable name for the test entity.

jlara310 force-pushed the regional-endpoint branch from d610131 to dc41325 Compare April 26, 2023 23:03

lesv suggested changes Apr 26, 2023

View reviewed changes

Use UUID for test entity. Fix linting errors.

52c15d5

lesv approved these changes May 1, 2023

View reviewed changes

jlara310 added 2 commits May 1, 2023 18:21

Fix linting error in javadoc @return comment

c446023

Fix order of imports in RegionalEndpointIT

66fd56a

Merge branch 'main' into regional-endpoint

99f1d99

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: added regional endpoint sample for datastore #1043

chore: added regional endpoint sample for datastore #1043

shweta345 commented Apr 10, 2023

snippet-bot bot commented Apr 10, 2023 •

edited

telpirion Apr 12, 2023

jlara310 Apr 12, 2023

telpirion Apr 12, 2023

jlara310 Apr 12, 2023

telpirion Apr 12, 2023

jlara310 Apr 12, 2023

telpirion Apr 12, 2023

jlara310 Apr 12, 2023

lesv left a comment

lesv Apr 26, 2023

jlara310 Apr 26, 2023

lesv Apr 26, 2023

jlara310 May 1, 2023

shweta345 May 9, 2023

shweta345 commented May 9, 2023


		System.out.printf("Retrieved %s: %s%n", taskKey.getName(), retrieved.getString("description"));

		systemsOutRule.assertContains("Saved sampletask1: Buy milk");

chore: added regional endpoint sample for datastore #1043

Are you sure you want to change the base?

chore: added regional endpoint sample for datastore #1043

Conversation

shweta345 commented Apr 10, 2023

snippet-bot bot commented Apr 10, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lesv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shweta345 commented May 9, 2023

snippet-bot bot commented Apr 10, 2023 •

edited