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

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

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

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

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:

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

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*

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

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

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

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

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

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

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

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