docs: samples and tests for auto-generated createDatabase and createInstance APIs. #2764
Conversation
…od while retrying exceptions in unit tests. * For details on issue see - googleapis#2206
fix: prevent illegal negative timeout values into thread sleep() method while retrying exceptions in unit tests.
product-auto-label
bot
added
size: l
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.example.spanner.v2; |
There was a problem hiding this comment.
I think that it is going to be confusing if we use .v2 as a package name for these samples, as the actual change is that the code will start using classes that are in a package called .v1, while they before that used classes that were in packages without a version number.
Or could we temporarily use this .v2 package, and when we make the switch, move these samples into com.example.spanner and the old samples into com.example.spanner.deprecated or something like that?
There was a problem hiding this comment.
Yeah as you described, I plan to use this path temporarily. Also instead of calling it .v2, I changed it to .admin.generated. Instead of calling the new path deprecated we can name it archived. So, the new samples will be moved into com.example.spanner and the old samples into com.example.spanner.archived
...les/snippets/src/main/java/com/example/spanner/v2/CreateDatabaseWithDefaultLeaderSample.java
Outdated
Show resolved
Hide resolved
...les/snippets/src/main/java/com/example/spanner/v2/CreateDatabaseWithDefaultLeaderSample.java
Outdated
Show resolved
Hide resolved
...les/snippets/src/main/java/com/example/spanner/v2/CreateDatabaseWithDefaultLeaderSample.java
Outdated
Show resolved
Hide resolved
|
Here is the summary of changes. You are about to add 2 region tags.
This comment is generated by snippet-bot.
|
…tabaseWithDefaultLeaderSample.java Co-authored-by: Knut Olav Løite <koloite@gmail.com>
...src/main/java/com/example/spanner/admin/generated/CreateDatabaseWithDefaultLeaderSample.java
Outdated
Show resolved
Hide resolved
samples/snippets/src/main/java/com/example/spanner/admin/generated/CreateInstanceExample.java
Outdated
Show resolved
Hide resolved
samples/snippets/src/test/java/com/example/spanner/admin/generated/SampleTestBaseV2.java
Show resolved
Hide resolved
|
I think it looks good from my end. One last thing came to mind when looking at the samples (which may or may not be a concern for Spanner). In google-cloud-java, we had some issues with creating instances for certain services. The project were running the code in had some quotas and the tests/samples had some issues properly deleting the created instances. We just ended up prefixing the resources with something like |
gcf-owl-bot
bot
removed
the
owlbot:run
| // TODO(developer): Replace these variables before running the sample. | ||
| final String instanceName = "projects/my-project/instances/my-instance-id"; | ||
| final String databaseId = "my-database-name"; | ||
| final String defaultLeader = "my-default-leader"; |
There was a problem hiding this comment.
nit: Could we use a default leader name here that is more similar to the actual value that a user would have to use. That might help users who don't know what the default leader option is, to understand and learn that.
samples/snippets/src/test/java/com/example/spanner/admin/generated/SampleTestBaseV2.java
Show resolved
Hide resolved
| } | ||
|
|
||
| databaseAdminClient.close(); | ||
| instanceAdminClient.close(); |
There was a problem hiding this comment.
The generated (admin) clients have both a close() and a shutdown() method. Do they do the same (or put another way: Does close() actually shutdown the client)?
There was a problem hiding this comment.
I don't think so. @lqiu96 Would you have a quick answer for this?
There was a problem hiding this comment.
I believe they're pretty much the same. Closing the client will close the underlying stub which closes all the BackgroundResources associated with the client. These are resources like channels, watchdog service, executors, etc.
The underlying implementation for the close() is to call shutdown() and I believe close() was originally offered as a way to utilize the client in try-with-resources
🤖 I have created a release *beep* *boop* --- ## [6.56.0](https://togithub.com/googleapis/java-spanner/compare/v6.55.0...v6.56.0) (2024-01-05) ### Features * Add autoscaling config in the instance to support autoscaling in systests ([#2756](https://togithub.com/googleapis/java-spanner/issues/2756)) ([99ae565](https://togithub.com/googleapis/java-spanner/commit/99ae565c5e90a2862b4f195fe64656ba8a05373d)) * Add support for Directed Read options ([#2766](https://togithub.com/googleapis/java-spanner/issues/2766)) ([26c6c63](https://togithub.com/googleapis/java-spanner/commit/26c6c634b685bce66ce7caf05057a98e9cc6f5dc)) * Update OwlBot.yaml file to pull autogenerated executor code ([#2754](https://togithub.com/googleapis/java-spanner/issues/2754)) ([20562d4](https://togithub.com/googleapis/java-spanner/commit/20562d4d7e62ab20bb1c4e78547b218a9a506f21)) ### Dependencies * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.21.0 ([#2772](https://togithub.com/googleapis/java-spanner/issues/2772)) ([173f520](https://togithub.com/googleapis/java-spanner/commit/173f520f931073c4c6ddf3b3d98d255fb575914f)) ### Documentation * Samples and tests for auto-generated createDatabase and createInstance APIs. ([#2764](https://togithub.com/googleapis/java-spanner/issues/2764)) ([74a586f](https://togithub.com/googleapis/java-spanner/commit/74a586f8713ef742d65400da8f04a750316faf78)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Notes