Re: Removing shading by default within BeamModulePlugin.groovy
I also noticed that the build takes significantly less time on my machine, several mins saved. On Fri, Jun 7, 2019 at 9:54 AM Lukasz Cwik wrote: > Guava was the only thing that we shaded everywhere but the original intent > was for us to shade more and more by default until we decided to do > vendoring (which is a better solution). > > So yes, this really only removed shading of Guava, we still have shading > in all these other places: > 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/* > > On Fri, Jun 7, 2019 at 1:05 AM Ismaël Mejía wrote: > >> 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 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 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 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: >>
Re: Removing shading by default within BeamModulePlugin.groovy
Guava was the only thing that we shaded everywhere but the original intent was for us to shade more and more by default until we decided to do vendoring (which is a better solution). So yes, this really only removed shading of Guava, we still have shading in all these other places: 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/* On Fri, Jun 7, 2019 at 1:05 AM Ismaël Mejía wrote: > 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 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 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 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: >
Re: Removing shading by default within BeamModulePlugin.groovy
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 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 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 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
Re: Removing shading by default within BeamModulePlugin.groovy
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 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 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 >> >
Re: Removing shading by default within BeamModulePlugin.groovy
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 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 >
Removing shading by default within BeamModulePlugin.groovy
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
