beam-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ismaël Mejía <ieme...@gmail.com>
Subject Re: Removing shading by default within BeamModulePlugin.groovy
Date Fri, 07 Jun 2019 08:04:43 GMT
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 <lcwik@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 <kenn@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 <lcwik@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

Mime
View raw message