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.

Sign up for GitHub

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

Jump to bottom

feat(spanner): add support for external hosts #3474

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+35 −1
Open

feat(spanner): add support for external hosts #3474

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1b5719
feat(spanner): add support for external hosts
sagnghos Nov 14, 2024
0a74b9c
feat(spanner): added unit test
sagnghos Nov 15, 2024
786e764
feat(spanner): added asserts to test
sagnghos Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseId.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ public static DatabaseId of(String project, String instance, String database) {
return new DatabaseId(new InstanceId(project, instance), database);
}

/** Creates a {@code DatabaseId} given instance and database IDs. */
public static DatabaseId of(String instance, String database) {
return new DatabaseId(new InstanceId("default", instance), database);
Copy link

Choose a reason for hiding this comment

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

I think we should document to users that this method assumes instance Id to be "default".

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe not instance Id, but rather project Id?

}

/** Creates a {@code DatabaseId} given the instance identity and database id. */
public static DatabaseId of(InstanceId instanceId, String database) {
return new DatabaseId(instanceId, database);
Expand Down
10 changes: 9 additions & 1 deletion google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ static class SessionId {
private static final PathTemplate NAME_TEMPLATE =
PathTemplate.create(
"projects/{project}/instances/{instance}/databases/{database}/sessions/{session}");
private static final PathTemplate EXTERNAL_HOST_NAME_TEMPLATE =
PathTemplate.create("instances/{instance}/databases/{database}/sessions/{session}");
private final DatabaseId db;
private final String name;

Expand All @@ -49,10 +51,16 @@ private SessionId(DatabaseId db, String name) {
static SessionId of(String name) {
Preconditions.checkNotNull(name);
Map<String, String> parts = NAME_TEMPLATE.match(name);
if (parts == null) {
parts = EXTERNAL_HOST_NAME_TEMPLATE.match(name);
Copy link

Choose a reason for hiding this comment

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

Can you add unit tests to cover this pattern and ensure the functionality works as intended?

Copy link
Author

Choose a reason for hiding this comment

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

Added unit test to cover existing pattern (for regression) as well as new pattern

}
Preconditions.checkArgument(
parts != null, "Name should conform to pattern %s: %s", NAME_TEMPLATE, name);
return of(
parts.get("project"), parts.get("instance"), parts.get("database"), parts.get("session"));
parts.containsKey("project") ? parts.get("project") : "default",
parts.get("instance"),
parts.get("database"),
parts.get("session"));
}

/** Creates a {@code SessionId} given project, instance, database and session IDs. */
Expand Down
Loading