Re: Vendoring / Shading Protobuf and gRPC

2018-07-19 Thread Lukasz Cwik
A lot of the package structure is a mess because we have a bunch of runner support code in various runner/* packages which was responsible for translating pipelines. I don't really have a strong opinion about whether the translation happens within sdks/java/core or another package under sdks/java/.

Re: Vendoring / Shading Protobuf and gRPC

2018-07-19 Thread Ismaël Mejía
Luke, thanks for explaining the idea, that’s what I expected, I was a bit confused after seeing that the proto transformation of the Pipeline happens in runners/core-contruction-java. I thought this was intended (maybe to keep the SDK smaller). So what is the future ? move all this machinery into s

Re: Vendoring / Shading Protobuf and gRPC

2018-07-18 Thread Ankur Goenka
FYI: The beam version just got updated today to 2.7-SNAPSHOT . This require re-adding the dependency jars for 2.7-SNAPSHOT version to the projects. On Wed, Jul 18, 2018 at 1:22 PM Lukasz Cwik wrote: > Are you delegat

Re: Vendoring / Shading Protobuf and gRPC

2018-07-18 Thread Lukasz Cwik
Are you delegating your test runs to Gradle or using Intellij's test runner? I have been delegating my test runs to Gradle and that has been working well for me. The platform test runner fails due to the missing classes as you mentioned. I also spent two hours trying to muck around with the Gradl

Re: Vendoring / Shading Protobuf and gRPC

2018-07-18 Thread Thomas Weise
Thanks for fixing the duplicate root issue, PR is merged. I cannot run RemoteExecutionTest from the same commit (Intellij v2018.1.6). Here are some of the class loading errors before I give up adding dependencies manually :( java.lang.NoClassDefFoundError: org/apache/beam/vendor/grpc/v1/io/grpc/B

Re: Vendoring / Shading Protobuf and gRPC

2018-07-18 Thread Lukasz Cwik
Ismael, the SDK should perform the pipeline translation to proto because I expect the flow to be: User Code -> SDK -> Proto Translation -> Job API -> Runner I don't expect "runners" to live within the users process anymore (excluding the direct runner). There will be one portable "runner" and it wi

Re: Vendoring / Shading Protobuf and gRPC

2018-07-17 Thread Thomas Weise
Thanks, the classpath order matters indeed. Still not able to run RemoteExecutionTest, but I was able to get the Flink portable test to work by adding the following to the *top* of the dependency list of *beam-runners-flink_2.11_test* vendor/sdks-java-extensions-protobuf/build/libs/beam-vendor-sd

Re: Vendoring / Shading Protobuf and gRPC

2018-07-17 Thread Ankur Goenka
Yes, I am able to run it. For tests, you also need to add dependencies to ":beam-runners-java-fn-execution/beam-runners-java-fn-execution_*test*" module. Also, I only added :beam-model-job-management-2.6.0-SNAPSHOT.jar :beam-model-fn-execution-2.6.0-SNAPSHOT.jar to the dependencies manually so no

Re: Vendoring / Shading Protobuf and gRPC

2018-07-17 Thread Thomas Weise
Are you able to run org.apache.beam.runners.fnexecution.control.RemoteExecutionTest from within Intellij ? I can get the compile errors to disappear by adding beam-model-job-management-2.6.0-SNAPSHOT.jar, io.grpc:grpc-core:1.12.0 and com.google.protobuf:protobuf-java:3.5.1 Running the test still

Re: Vendoring / Shading Protobuf and gRPC

2018-07-17 Thread Ankur Goenka
For reference: I was able to make intellij work with the master by doing following steps 1. Remove module :beam:vendor-sdks-java-extensions-protobuf from intellij. 2. Adding :beam-model-fn-execution/build/libs/beam-model-fn-execution-2.6.0-SNAPSHOT.jar and :beam-model-job-manageme

Re: Vendoring / Shading Protobuf and gRPC

2018-07-17 Thread Thomas Weise
Adding the external jar in Intellij (2018.1) currently fails due to a duplicate source directory (sdks/java/extensions/protobuf/src/main/java). The build as such also fails, with: error: warnings found and -Werror specified Ismaël found removing https://github.com/apache/beam/blob/master/buildSr

Re: Vendoring / Shading Protobuf and gRPC

2018-07-12 Thread Ismaël Mejía
Seems reasonable, but why exactly may we need the model (or protobuf related things) in the future in the SDK ? wasn’t it supposed to be translated into the Pipeline proto representation via the runners (and in this case the dep reside in the runner side) ? On Thu, Jul 12, 2018 at 2:50 AM Lukasz Cw

Re: Vendoring / Shading Protobuf and gRPC

2018-07-11 Thread Lukasz Cwik
Got a fix[1] for Andrews issue which turned out to be a release blocker since it broke performing the release. Also fixed several minor things like javadoc that were wrong with the release. Solving it allowed me to do the publishing in parallel and cut the release time from 20+ mins to 8 mins on my

Re: Vendoring / Shading Protobuf and gRPC

2018-07-11 Thread Andrew Pilloud
We discussed this in person, sounds like my issue is known and will be fixed shortly. I'm running builds with '-Ppublishing' because I need to generate release artifacts for bundling the Beam SQL shell with the Google Cloud SDK. Hope to eventually just use the Beam release, but we are currently cut

Re: Vendoring / Shading Protobuf and gRPC

2018-07-11 Thread Lukasz Cwik
Andrew, to my knowledge it seems as though your running into BEAM-4744, is there a reason you need to specify -Ppublishing? No particular reason to using ByteString within ByteKey and TextSource. Note that we currently do shade away protobuf in sdks/java/core so we could either migrate to using a

Re: Vendoring / Shading Protobuf and gRPC

2018-07-11 Thread Jean-Baptiste Onofré
That's really cool ! I'm thinking about the Spark runner where grpc deps are not easy to deal with. Thanks ! Regards JB On 10/07/2018 23:09, Lukasz Cwik wrote: > With the merge of PR #5594[1], we started shading all gRPC / Protobuf > dependencies within all the modules that depended on the model

Re: Vendoring / Shading Protobuf and gRPC

2018-07-11 Thread Ismaël Mejía
This is great news in particular for runners (Spark) where the leaking of some grpc subdependencies caused stability issues and required extra shading. Great ! About the other modules > Note, these are the following modules that still depend on protobuf that are shaded away and could move to use

Re: Vendoring / Shading Protobuf and gRPC

2018-07-11 Thread Andrew Pilloud
This is really cool and should cut down our artifact size significantly! Thanks Luke! I am running into one issue after this: builds with the publishing flag no longer work. (We run './gradlew -Ppublishing shadowJar' to generate release artifacts for the Beam SQL shell.) I get a bunch of errors li

Vendoring / Shading Protobuf and gRPC

2018-07-10 Thread Lukasz Cwik
With the merge of PR #5594[1], we started shading all gRPC / Protobuf dependencies within all the modules that depended on the model/* dependencies by vendoring them. The vendored versions are built and packaged into the model jars (they should be separated out once I figure out how to generate pro