This is fantastic. Took a look at the PR and did not see anything that
jump to my eyes and also validated with two external projects with
today’s snapshots (after merge) without issues so far. Great that we
finally tackle this on, thanks Luke!

Have one minor comment because the title of the thread may be
confusing, after checking sdks-java-core I noticed we are still
shading other dependencies: protobuf, bytebuddy, antlr, apache
commons, so I suppose this was mostly around shading guava, isn’t it?

On Wed, Jun 5, 2019 at 10:09 PM Lukasz Cwik <lc...@google.com> wrote:
>
> I am able to pass several runners validates runner tests and the Java 
> PostCommit.
>
> I also was able to publish a release into the staging repository[1] and 
> compared the newly generated poms artifact-2.14.0-20190605.*-30.pom against 
> the previously nightly snapshot of artifact-2.14.0-20190605.*-28.pom for the 
> following projects as a spot check and found no differences in those poms:
> beam-sdks-java-core
> beam-sdks-java-fn-execution
> beam-runners-spark
>
> I believe my PR is now ready for review.
>
> 1: https://repository.apache.org/content/groups/snapshots/org/apache/beam/
>
> On Tue, Jun 4, 2019 at 7:18 PM Kenneth Knowles <k...@apache.org> wrote:
>>
>> Nice! This is a huge step. One thing that showed up in the last big gradle 
>> change was needing to check the generated poms.
>>
>> Kenn
>>
>> On Tue, Jun 4, 2019 at 5:07 PM Lukasz Cwik <lc...@google.com> wrote:
>>>
>>> Since we have been migrating to using vendoring instead of shading[1] and 
>>> due to previous efforts in vendoring[2, 3] I have opened up PR 8762[4] 
>>> which migrates all projects that weren't doing anything shading wise to not 
>>> perform any shading. This required me to fix up all intra project 
>>> dependencies and release publishing.
>>>
>>> The following is a list of all project paths which are still using shading 
>>> for some reason:
>>> model/*
>>> sdks/java/core
>>> sdks/java/extensions/kryo
>>> sdks/java/extensions/sql
>>> sdks/java/extensions/sql/jdbc
>>> sdks/java/harness
>>> runners/spark/job-server
>>> runners/direct-java
>>> runners/samza/job-server
>>> runners/google-cloud-dataflow-java/worker
>>> runners/google-cloud-dataflow-java/worker/legacy-worker
>>> runners/google-cloud-dataflow-java/worker/windmill
>>> vendor/*
>>>
>>> Out of the list above, migrating sdks/java/core and runners/direct-java (in 
>>> that order) would provide the most benefit to moving away from shading 
>>> within our project. Many of the others are either shaded proto classes or 
>>> applications (e.g. job-servers, harness, sql jdbc) and either require 
>>> shading to be compatible with vendoring or aren't meant to be used as 
>>> dependencies.
>>>
>>> Since this is a larger change that cuts across so many projects there is 
>>> risk for breakage. I'm looking for people to help test the change and 
>>> validate any scenarios that they are specifically interested in. I'm 
>>> planning to run several of the postcommits on my PR and check that we can 
>>> build a release in addition to any efforts others provide before looking to 
>>> have the change merged.
>>>
>>> The following guidance should help those who edit Gradle build files (after 
>>> this change is merged):
>>> * For projects that don't perform any shading, those projects have been 
>>> migrated to use the default configurations that the Gradle Java plugin 
>>> uses[5]. Note that the default configurations we use have been deprecated.
>>> * For projects that depend on another project that isn't shaded, the intra 
>>> project configuration has been swapped to use compile / testRuntime instead 
>>> of shadow and shadowTest
>>> * Existing projects that are still shaded should use the shadow and 
>>> shadowTest configurations as before.
>>>
>>> 1: 
>>> https://lists.apache.org/thread.html/4c12db35b40a6d56e170cd6fc8bb0ac4c43a99aa3cb7dbae54176815@%3Cdev.beam.apache.org%3E
>>> 2: 
>>> https://lists.apache.org/thread.html/4c12db35b40a6d56e170cd6fc8bb0ac4c43a99aa3cb7dbae54176815@%3Cdev.beam.apache.org%3E
>>> 3: 
>>> https://lists.apache.org/thread.html/972b5175641f4eaf7ec92870cc0ff72fa52e6f0bbaccc384a3814e45@%3Cdev.beam.apache.org%3E
>>> 4: https://github.com/apache/beam/pull/8762
>>> 5: 
>>> https://docs.gradle.org/current/userguide/java_plugin.html#sec:java_plugin_and_dependency_management

Reply via email to