mango icon indicating copy to clipboard operation
mango copied to clipboard

GA4GH grpc/pb compatibility errors with bdgenomics/Spark

Open jpdna opened this issue 8 years ago • 6 comments

I want to use the GA4GH schema protobuf generated java classes for purposes of enabling Mango/bdgenomics GA4GH services.

Until the Maven objects are available at Maven Central, I want to install to local mvn repo by: git clone https://github.com/ga4gh/ga4gh-schemas ga4gh-schemas paschalj$ mvn clean install after which in Mango I can depend on with:

      <dependency>
        <groupId>org.ga4gh</groupId>
        <artifactId>ga4gh-schemas</artifactId>
        <version>0.6.0a10-SNAPSHOT</version>
      </dependency>

I did change the jdk dependency to 1.8 and the proto-version to 3.2.0 in ga4gh-schemas pom in an attempt to solve problem below, but no succcess.

Using the ga4gh-schema dependency Mango compile succeeds, but when I try to test code inside Mango:

import ga4gh.VariantServiceOuterClass
import ga4gh.VariantServiceOuterClass.SearchVariantsRequest
import ga4gh.VariantServiceOuterClass.SearchVariantsRequest.Builder
val x: Builder = VariantServiceOuterClass.SearchVariantsRequest.newBuilder()
x.setStart(223)
val z: SearchVariantsRequest = x.build()

I get the following run time error:

java.lang.NoSuchMethodError: com.google.protobuf.Descriptors$Descriptor.getOneofs()Ljava/util/List;
Exception in thread "main" java.lang.NoSuchMethodError: com.google.protobuf.Descriptors$Descriptor.getOneofs()Ljava/util/List;
	at com.google.protobuf.GeneratedMessageV3$FieldAccessorTable.<init>(GeneratedMessageV3.java:1707)
	at com.google.protobuf.StructProto.<clinit>(StructProto.java:78)
	at ga4gh.Common.<clinit>(Common.java:13705)
	at ga4gh.Variants.<clinit>(Variants.java:11046)
	at ga4gh.VariantServiceOuterClass.<clinit>(VariantServiceOuterClass.java:8526)

I suspect this is some sort of version conflict issue, but not sure where to go next. Something like: https://github.com/nats-io/java-nats-streaming/issues/20

If anyone can successfully locally generate java maven artifact from: https://github.com/ga4gh/ga4gh-schemas and use as a dependency in bdgenomics ADAM/Mango project I'd love to take a look at a branch where you do that.

Side note: While from Mango we will in the end just be constructing JSON request objects to POST and parsing returned JSON it would be preferable to directly make use of the PB based code-generated objects I think (let me know if you think otherwise).

May be of interest to @david4096 @kozbo @fnothaft @heuermh @akmorrow13

jpdna avatar Mar 15 '17 13:03 jpdna

I used two different methods for generating Java code from Protobuf schemas here https://github.com/heuermh/phenopacket-protobuf

The code generated by Google's stuff is pretty bad (at least at version 3.0.0-beta-3 as used there) compared to what the https://github.com/square/wire library generates; I'm not sure about version compatibility with ga4gh though.

heuermh avatar Mar 15 '17 13:03 heuermh

I seem to have solved this by shading protobuf inside ga4gh, adding below to https://github.com/ga4gh/ga4gh-schemas/blob/master/pom.xml

<plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-shade-plugin</artifactId>
        <version>2.4.3</version>
        <executions>
          <execution>
            <phase>package</phase>
            <goals>
              <goal>shade</goal>
              </goals>
            <configuration>
              <relocations>
                <relocation>
                  <pattern>com.google.protobuf</pattern>
                  <shadedPattern>shaded.ga4gh.com.google.protobuf</shadedPattern>
                  </relocation>
                </relocations>
              </configuration>
            </execution>
          </executions>
        </plugin>

jpdna avatar Mar 15 '17 14:03 jpdna

However I am just not familiar enough to know if its a good idea for ga4gh-schema Master to accept a PR with this shaded protobuf change. If bdgenomics ADAM/Mango wants to use a maven central artifact in the way I currently did successfully, then would we need this shading change to be accepted upstream in ga4gh-schema?

jpdna avatar Mar 15 '17 14:03 jpdna

Always grateful @jpdna. We can move that to a more recent protobuf release.

You may find the pom.xml in https://github.com/ga4gh/compliance useful. However, it doesn't use any grpc features and might be a waste of time.

david4096 avatar Mar 15 '17 14:03 david4096

Hi @jpdna! Can you push the branch where you're working? I'd like to play around with the pom to see if we can shade the upstream transitive dependency. I think we should be able to shade it in the mango überjar, which would make this all simple.

Also, I hate to always propose batty insane stuff, but if we can't shade an upstream dependency easily, I think we could do something that is a variant off of what I did in https://github.com/bigdatagenomics/adam/pull/1391? However, I'd like to see if we can do things the "easy" way before trying that.

fnothaft avatar Mar 15 '17 15:03 fnothaft

My ga4gh branch of Mango is here: https://github.com/jpdna/mango/tree/ga4gh

I put in test code that uses a ga4gh-schema class in VizReads.scala https://github.com/jpdna/mango/blob/ga4gh/mango-cli/src/main/scala/org/bdgenomics/mango/cli/VizReads.scala#L713

such that it will run and attempt to use ga4gh-schema if you run: $MANGO_HOME/example-files/run-example.sh

Again, the above failed until I modified the ga4gh-schema pom to shade com.google.protobuf but if we can find a way to do that from within Mango ourselves as you suggest that would be better.

jpdna avatar Mar 15 '17 16:03 jpdna