Initial gRPC work for vector query service#1269
Conversation
67290f4 to
fa78aef
Compare
4 similar comments
fa78aef to
24a935c
Compare
| @@ -0,0 +1,624 @@ | |||
| // Generated by the protocol buffer compiler. DO NOT EDIT! | |||
There was a problem hiding this comment.
do we have a good reason to include generated code? I'm 99.9% against including generated code in version control, unless there's something that necessitates it.
| <version>${project.version}</version> | ||
| </dependency> | ||
| </dependencies> | ||
| <!-- <build> |
There was a problem hiding this comment.
cleanup commented out blocks
| server.blockUntilShutdown(); | ||
| } | ||
| catch (final Exception e) { | ||
| LOGGER.error(e.getMessage()); |
| responseObserver.onCompleted(); | ||
| } | ||
| catch (final Exception e) { | ||
| LOGGER.error(e.getMessage()); |
There was a problem hiding this comment.
log the exception (stack trace)
| JTSFactoryFinder.getGeometryFactory()).read(geomDefinition); | ||
| } | ||
| catch (final Exception e) { | ||
| LOGGER.error(e.getMessage()); |
|
|
||
| final ByteArrayId adapterId = new ByteArrayId( | ||
| request.getBaseParams().getAdapterId().toByteArray()); | ||
| final ByteArrayId indexId = new ByteArrayId( |
There was a problem hiding this comment.
indexID and adapter ID are typically optional params on a query
There was a problem hiding this comment.
Leaving these parameters as discussed. They are indeed optional as well when invoking the gRPC call.
| <modelVersion>4.0.0</modelVersion> | ||
| <groupId>mil.nga.giat</groupId> | ||
| <artifactId>geowave-grpc-protobuf-shaded</artifactId> | ||
| <version>0.0.1-SNAPSHOT</version> |
There was a problem hiding this comment.
also, I think we should version this with geowave versions (just make it use the geowave parent)
24a935c to
6d7e3d5
Compare
6d7e3d5 to
bc8e6e7
Compare
| @@ -0,0 +1,23 @@ | |||
| package mil.nga.giat.geowave.service.grpc.protobuf.util; | |||
There was a problem hiding this comment.
It is used by gRPC clients when making a call through the stub. Otherwise, we have another reliance on protobuf's ByteString (this is part of the shade/relocation strategy). In the future I believe we should actually look at using a shaded version of hbase Kent and I came across. It shades it's usage of protobuf 2.5 - that way anything moving forward can just include protobuf 3 no problem. At the moment we are kinda doing things backward (everything using protobuf 3 needs to shade at the moment).
| <version>0.9.7-SNAPSHOT</version> | ||
| </parent> | ||
| <artifactId>geowave-grpc-protobuf-shaded</artifactId> | ||
| <version>0.0.1-SNAPSHOT</version> |
| LOGGER.error("Exception encountered:", e.getStackTrace().toString()); | ||
| } | ||
|
|
||
| final QueryOptions options = new QueryOptions( |
There was a problem hiding this comment.
are we handling empty strings for adapter/index IDs?
There was a problem hiding this comment.
Yes if the string is empty I set the variable to null actually that way the type is explicit instead of accidentally passing in empty string like we discussed.
There was a problem hiding this comment.
oh, missed it on line 205-208
| <version>0.0.1-SNAPSHOT</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.google.guava</groupId> |
There was a problem hiding this comment.
yikes, new versions of guava are scary - but if this is tested with HBase we should be good
There was a problem hiding this comment.
I agree but we need the newer version for gRPC, so far it works ok with hbase.
| "ignored"); // the name is not used in this case | ||
| } | ||
| catch (final Exception e) { | ||
| LOGGER.error("Exception encountered:", e.getStackTrace().toString()); |
There was a problem hiding this comment.
I had meant something like LOGGER.error("Exception encountered reading geometry", e);
bc8e6e7 to
67614e6
Compare
3 similar comments
| <dependency> | ||
| <groupId>mil.nga.giat</groupId> | ||
| <artifactId>geowave-grpc-protobuf-shaded</artifactId> | ||
| <version>0.0.1-SNAPSHOT</version> |
There was a problem hiding this comment.
shouldn't this be ${project.version} now that the shaded artifact is part of the same versioning as the rest of geowave?
67614e6 to
76293ba
Compare
This work includes a package for shading the parts of gRPC/protobuf 3 that conflict with imports from 2.5