GCD 0.1.7 release incompatible with v1beta3 emulator

[tom-haines]

I have posted repeatable tests to demonstrate the regressions here:
github.com/tom-haines/gcd-dot-one-seven-regression

There are various errors when trying to communicate with the latest release of the gcd emulator (v1beta3) that prevents 0.1.7 from being used with v1beta3. The same basic "connect and push tests" all work under 0.1.6.

I've included examples of the regression under both docker and localhost; using DNS lookups and IPv4 addressing. There are also scheme and scheme-less variants of the URL used.

[eddavisson]

Looks like it's calling /v1beta3/projects/sample-ds instead of /datastore/v1beta3/projects/sample-ds.

@ajkannan What's the correct way to configure gcloud-java to talk to the local emulator?

[eddavisson]

By the way, thank you for providing code for the repro!

[ajkannan]

@eddavisson, this looks like a problem of conflicting constraints in the v1beta3 client. When specifying a localhost, scheme is not allowed. As a result, gcloud-java strips scheme from user input. However, the check in DatastoreFactory's validateUrl fails when the input starts with "127.0.0.1" (an "http://" is necessary for IP addresses). As a result, input using IP addresses rather than hostnames will not work. gcloud-java uses "localhost" instead of "127.0.0.1", so we didn't encounter this problem in our tests. A fix will have to come in the client for IP addresses to work. Depending on which constraint is relaxed in the client, gcloud-java may also have to push a fix.

@tom-haines Thanks for pointing this out and providing repro code! I would like to make a suggestion if you are using code similar to your repro code to set up your own tests. gcloud-java's LocalDatastoreHelper class makes it simpler to set up the tests against the local emulator. (Note: this helper was renamed and refactored right after 0.1.7; we recommend you upgrade to the latest gcloud-java version 0.2.1. If you'd like to stay with 0.1.7, the relevant class is LocalGcdHelper). gcloud-java's test helper will eliminate the need to download and start the gcd script yourself, and it also has an options() method that returns options so that you don't have to construct them yourself. (see the package-info for an example).

[eddavisson]

I think the issue might be that gcloud-java's options object accepts a single host option and then uses a heuristic to decide whether to set the "local host" option or the "project endpoint" on the underlying DatastoreOptions object. If the IP were recognized as a "local host", it would set the "local host" option, and then this library's DatastoreFactory takes care of adding the scheme (and /datastore in the path).
https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/spi/DefaultDatastoreRpc.java#L42

The "local host" option is a little awkward, but it provides an additional hint to our client not to try to get credentials from the local environment (which can be slow).

A couple of options come to mind off the top of my head:

1. Split out a separate "local host" option in gcloud-java.
2. Always pass raw IP addresses to this library's "local host" option.

[ajkannan]

If the IP were recognized as a "local host", it would set the "local host" option, and then this library's DatastoreFactory takes care of adding the scheme (and /datastore in the path).

When a user sets the host to "127.0.0.1:8000", gcloud-java correctly determines that the user is trying to set the local host. For that reason, I don't think splitting up the local host option in gcloud-java or passing IP addresses to the local host option would solve this particular problem.

To clarify what I was saying before, it's not possible to supply raw IP addresses to the local host option in this library because validateUrl requires scheme (new URI("some IP address without scheme") will throw an exception) but this library's DatastoreOptions won't accept scheme for localhost.

[eddavisson]

Fixed in 1.0.0-beta.2.

[email2liyang]

when will v1.0.0 released?

[ajkannan]

Hi @email2liyang, are you referring to v1.0.0 for the gcloud-java client library or version 1.0.0-beta.2 for Cloud Datastore's client (the client in this repository)? If you're asking about gcloud-java, I suggest opening an issue on that repository (located here), and somebody should get back to you on that. If you meant the latter, Cloud Datastore's client (which gcloud-java currently depends on) is available on Maven now (see here).

I just made a pull request to update gcloud-java with this fix (see googleapis/google-cloud-java#1002). Once that gets merged in and gcloud-java releases v0.2.2, you will be able to use IP addresses corresponding to your local machine in DatastoreOptions. My understanding is that waiting for the next gcloud-java release shouldn't be a blocker, since you can use hostnames (or even better, use options() in LocalDatastoreHelper as described in my previous post), but correct me if that's not the case.

[tom-haines]

Thanks @ajkannan for the prompt fix (and note your PR has now been merged into gcloud-java).

Regarding whether this is a blocker re use of hostnames: I don't think you can use hostnames as suggested, and hence remains a blocker for using v0.1.7 with the current emulator.

In regression repo, have a look at the last test (copied here):

Even when using a hostname with the emulator (here hardwired hostname "docker-host" to docker-machine ip dev in /etc/hosts )

mvn clean test -Dgcp_ip=docker-host

Will FAIL.

But will PASS under 0.1.6

mvn -f pom-0.1.6.xml clean test -Dgcp_ip=docker-host

example config:

$ docker-machine ip dev
172.16.10.157
$ cat /etc/hosts
172.16.10.157 docker-host

( I've mentioned "docker-host" here, but equally you could just run gcd.sh on any machine of course )

[ajkannan]

Hi @tom-haines, this happens to be a distinct issue from the one above. I've opened an issue (googleapis/google-cloud-java#1014) to track fixing it. The issue here is that the IP address for the Docker container isn't recognized as a local or loopback socket, so gcloud-java considers it a "project endpoint" rather than a local host. This means two things:

1. You should use scheme in the host URL (in 0.1.7, scheme is not allowed for local host, but is required for project endpoints).
2. The version of the gcd script you're running (and the version that gcloud-java uses right now) uses a slightly different URL construction than the actual Google Cloud Datastore URL. It requires an extra "/datastore" (e.g. http://<hostname>:<port>/datastore). This is going away in the next version of the local emulator. gcloud-java plans to add a fix so that your Docker example (provided you add scheme to the host URL) should work. This fix depends on both this library's client being updated and the next version of gcloud-java being released.

If you need a quick workaround to upgrade gcloud-java and communicate with gcd.sh running in a container, use the URL http://<hostname or IP>:<port>/datastore instead of <hostname>:<port> when constructing DatastoreOptions. Keep in mind you'll have to change the URL again to http://<hostname or IP>:<port> when you update to the future release of gcloud-java that contains the fix for googleapis/google-cloud-java#1014.

[email2liyang]

@ajkannan , I find another incompatible error when upgrading from v0.1.6 to v0.2.2
if you execute method below in v0.1.6, it works without problem, when upgrade to v0.2.2,it throws exception"java.lang.IllegalArgumentException: Could not parse key". we have millions of record stored in other system for google key, this error is blocking us to upgrade to latest version of gcloud-java.

could you pls fix? or is there any way we can convert these type of String into a parseable key in latest version of gcloud-java?

com.google.cloud.datastore.Key Key
        googleKey =
        Key.fromUrlSafe(
            "partition_id+%7B%0A++dataset_id%3A+%22datastore-test-1123%22%0A%7D%0Apath_element+%7B%0A++kind%3A+%22ip_pending_msg%22%0A++id%3A+5629499534213120%0A%7D%0A");

anything incompatible from the implementation of https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/Key.java?

[email2liyang]

@ajkannan , here is an example to re-produce this issue

https://github.com/email2liyang/pi-google-datastore

[ajkannan]

Hi @email2liyang, thanks for providing repro code. The issue here is that the keys were encoded using the v1beta2 key definition, and the current Datastore version (v1beta3, which is used in gcloud-java v0.2.2) changed the key definition. gcloud-java's toUrlSafe and fromUrlSafe methods use the proto definition to encode keys, so fromUrlSafe in v1beta3 won't be able to decipher a v1beta2 URL-encoded key. For example, if you change "dataset_id"-->"project_id" and "path_element"-->"path" in the URL key you provided in your sample code, you'll get a valid key in gcloud-java v0.2.2.

To fix your problem, I recommend the following steps:

1. Depend on gcloud-java v0.2.2, and add a dependency to your pom.xml for the v1beta2 proto definition. The Datastore team changed the package name between v1beta2 and v1beta3, so it's safe to depend on both versions.

2. Convert the v1beta2 URL-safe key strings you have stored to v1beta3 keys. To do this, create a DatastoreV1.Key (the v1beta2 key) and transfer over the fields to the v1beta3 key. I added some code below that isn't thoroughly tested, but should give you an idea of what needs to be done.

   import static java.nio.charset.StandardCharsets.UTF_8;
   
   import com.google.api.services.datastore.DatastoreV1;
   import com.google.cloud.datastore.Key;
   import com.google.cloud.datastore.PathElement;
   import com.google.protobuf.TextFormat;
   
   import java.io.UnsupportedEncodingException;
   import java.net.URLDecoder;
   import java.util.ArrayList;
   import java.util.List;
   
   public static Key fromV1Beta2UrlSafe(String urlSafe) {
     DatastoreV1.Key old;
     try {
       String utf8Str = URLDecoder.decode(urlSafe, UTF_8.name());
       DatastoreV1.Key.Builder builder = DatastoreV1.Key.newBuilder();
       TextFormat.merge(utf8Str, builder);
       old = builder.build();
     } catch (UnsupportedEncodingException e) {
       throw new IllegalStateException("Unexpected decoding exception", e);
     } catch (TextFormat.ParseException e) {
       throw new IllegalArgumentException("Could not parse key", e);
     }
     List<DatastoreV1.Key.PathElement> path = old.getPathElementList();
     DatastoreV1.Key.PathElement leaf = path.get(path.size() - 1);
     Key.Builder newKey;
     if (leaf.hasName()) {
       newKey = Key.builder(old.getPartitionId().getDatasetId(), leaf.getKind(), leaf.getName());
     } else {
       newKey = Key.builder(old.getPartitionId().getDatasetId(), leaf.getKind(), leaf.getId());
     }
     newKey.namespace(old.getPartitionId().getNamespace());
     List<DatastoreV1.Key.PathElement> oldPath = old.getPathElementList();
     List<PathElement> ancestors = new ArrayList<PathElement>(oldPath.size() - 1);
     for (DatastoreV1.Key.PathElement pathElement : oldPath) {
       if (pathElement.hasName()) {
         ancestors.add(PathElement.of(pathElement.getKind(), pathElement.getName()));
       } else {
         ancestors.add(PathElement.of(pathElement.getKind(), pathElement.getId()));
       }
     }
     newKey.ancestors(ancestors);
     return newKey.build();
   }

3. You can use the v1beta3 key with gcloud-java 0.2.2 methods. You can also create a v1beta3-compatible URL-safe key string to save in your records for future use.

You'll either need to take those steps in some sort of bulk operation (such as a map reduce) to convert all the records you have, or you can lazily convert keys to the new specification when you need to use them.

[email2liyang]

Hi, @ajkannan , thanks for the quick reply, I'm a little bit concern about adding dependency of v1beta2 , as it will introduce some duplicated lib into the project which may cause other unknown issue
e.g:
google-http-client and google-oauth-client.

can google datastore team can fix the issue inside the new version of code which will respect old version but do not introduce so many duplicated dependencies?

 --- maven-dependency-plugin:2.8:tree (default-cli) @ incompatible ---
  [INFO] io.practiceinsight:incompatible:jar:1.0-SNAPSHOT
  [INFO] +- com.google.cloud:gcloud-java:jar:0.2.2:compile
  [INFO] |  +- com.google.cloud:gcloud-java-bigquery:jar:0.2.2:compile
  [INFO] |  |  \- com.google.apis:google-api-services-bigquery:jar:v2-rev270-1.21.0:compile
  [INFO] |  +- com.google.cloud:gcloud-java-compute:jar:0.2.2:compile
  [INFO] |  |  \- com.google.apis:google-api-services-compute:jar:v1-rev103-1.21.0:compile
  [INFO] |  +- com.google.cloud:gcloud-java-core:jar:0.2.2:compile
  [INFO] |  |  +- com.google.auth:google-auth-library-credentials:jar:0.3.1:compile
  [INFO] |  |  +- com.google.auth:google-auth-library-oauth2-http:jar:0.3.1:compile
  [INFO] |  |  |  \- com.google.http-client:google-http-client-jackson2:jar:1.19.0:compile
  [INFO] |  |  |     \- com.fasterxml.jackson.core:jackson-core:jar:2.1.3:compile
  [INFO] |  |  +- com.google.api-client:google-api-client-appengine:jar:1.21.0:compile
  [INFO] |  |  |  +- com.google.oauth-client:google-oauth-client-appengine:jar:1.21.0:compile
  [INFO] |  |  |  |  +- com.google.oauth-client:google-oauth-client-servlet:jar:1.21.0:compile
  [INFO] |  |  |  |  |  \- com.google.http-client:google-http-client-jdo:jar:1.21.0:compile
  [INFO] |  |  |  |  \- javax.servlet:servlet-api:jar:2.5:compile
  [INFO] |  |  |  +- com.google.api-client:google-api-client-servlet:jar:1.21.0:compile
  [INFO] |  |  |  |  \- javax.jdo:jdo2-api:jar:2.3-eb:compile
  [INFO] |  |  |  |     \- javax.transaction:transaction-api:jar:1.1:compile
  [INFO] |  |  |  \- com.google.http-client:google-http-client-appengine:jar:1.21.0:compile
  [INFO] |  |  +- joda-time:joda-time:jar:2.9.2:compile
  [INFO] |  |  \- org.json:json:jar:20151123:compile
  [INFO] |  +- com.google.cloud:gcloud-java-datastore:jar:0.2.2:compile
  [INFO] |  |  +- com.google.cloud.datastore:datastore-v1beta3-protos:jar:1.0.0-beta:compile
  [INFO] |  |  \- com.google.cloud.datastore:datastore-v1beta3-proto-client:jar:1.0.0-beta.2:compile
  [INFO] |  +- com.google.cloud:gcloud-java-dns:jar:0.2.2:compile
  [INFO] |  |  +- com.google.apis:google-api-services-dns:jar:v1-rev7-1.21.0:compile
  [INFO] |  |  \- commons-fileupload:commons-fileupload:jar:1.3.1:compile
  [INFO] |  |     \- commons-io:commons-io:jar:2.2:compile
  [INFO] |  +- com.google.cloud:gcloud-java-resourcemanager:jar:0.2.2:compile
  [INFO] |  |  \- com.google.apis:google-api-services-cloudresourcemanager:jar:v1beta1-rev10-1.21.0:compile
  [INFO] |  \- com.google.cloud:gcloud-java-storage:jar:0.2.2:compile
  [INFO] |     \- com.google.apis:google-api-services-storage:jar:v1-rev62-1.21.0:compile
  [INFO] +- com.google.apis:google-api-services-datastore-protobuf:jar:v1beta2-rev1-4.0.0:compile
  [INFO] |  +- com.google.protobuf:protobuf-java:jar:3.0.0-beta-1:compile
  [INFO] |  +- com.google.http-client:google-http-client:jar:1.15.0-rc:compile
  [INFO] |  |  +- com.google.code.findbugs:jsr305:jar:1.3.9:compile
  [INFO] |  |  +- org.apache.httpcomponents:httpclient:jar:4.0.1:compile
  [INFO] |  |  |  +- org.apache.httpcomponents:httpcore:jar:4.0.1:compile
  [INFO] |  |  |  +- commons-logging:commons-logging:jar:1.1.1:compile
  [INFO] |  |  |  \- commons-codec:commons-codec:jar:1.3:compile
  [INFO] |  |  \- xpp3:xpp3:jar:1.1.4c:compile
  [INFO] |  +- com.google.http-client:google-http-client-protobuf:jar:1.15.0-rc:compile
  [INFO] |  +- com.google.http-client:google-http-client-jackson:jar:1.15.0-rc:compile
  [INFO] |  |  \- org.codehaus.jackson:jackson-core-asl:jar:1.9.11:compile
  [INFO] |  +- com.google.oauth-client:google-oauth-client:jar:1.15.0-rc:compile
  [INFO] |  +- com.google.api-client:google-api-client:jar:1.15.0-rc:compile
  [INFO] |  \- com.google.guava:guava:jar:12.0:compile

[ajkannan]

@email2liyang, the duplicated dependencies you mentioned can be explicitly excluded via the pom.

Adding this logic in gcloud-java would be less than ideal, because we'd use a similar approach as mentioned above (depend on the v1beta2 libraries and exclude unnecessary transitive dependencies). However, then we'd be adding an extra (deprecated) dependency for all our users going forward. On the other hand, if you run a bulk operation to update your records, then you no longer need the v1beta2 dependency.