I spent all of yesterday investigating and fixing dependency issues outside of SQL. I really regret the decision to write a test for this. Would it be acceptable for us to put testing with the output jar behind a flag like we do for failOnWarning?
On Wed, May 23, 2018 at 5:21 PM Kenneth Knowles <k...@google.com> wrote: > What's the status of moving it forward? Is it a ton of work / too much to > do quickly? > > On Wed, May 23, 2018 at 9:11 AM Andrew Pilloud <apill...@google.com> > wrote: > >> To loop the list in on discussions going on in >> https://github.com/apache/beam/pull/5443: our normal tests don't run >> against the shaded jars. Gradle can run the tests against the shaded jars, >> but a bunch fail due to dependency issues. It's not just SQL. >> >> Andrew >> >> On Mon, May 21, 2018 at 11:35 AM Lukasz Cwik <lc...@google.com> wrote: >> >>> Shading requires two pieces of information: >>> 1) Which dependencies should be part of the shaded jar (controlled by >>> includes/excludes) >>> 2) How to relocate code within those dependencies (controlled by >>> relocations) >>> >>> The reason why the exclude(".*") exists is because typically it is an >>> error to produce a shaded package with dependencies which are not >>> relocated. When libraries do this, it causes a lot of >>> NoClassFound/NoMethodFound errors for users since a user can't know which >>> version of a dependency they are actually getting (the one that was bundled >>> part of your jar or the one they depend on as a library). Only applications >>> should ever really do this, libraries should always repackage all their >>> code to prevent such errors. >>> >>> Note that in the SQL package, you can provide your own shadowClosure to >>> the applyJavaNature() which means that the default won't apply. For >>> example: >>> https://github.com/apache/beam/blob/a3ba6a0e8de3ae72b8fc6fc6038eb9dc725f092e/sdks/java/harness/build.gradle#L20 >>> and remove the 'DEFAULT_SHADOW_CLOSURE <<' >>> >>> >>> On Mon, May 21, 2018 at 10:26 AM Andrew Pilloud <apill...@google.com> >>> wrote: >>> >>>> The issue SQL is seeing is caused by a default dependency of >>>> exclude(".*") added in build_rules.gradle. This breaks the normal method of >>>> building shadow jars as everything must be explicitly included. SQL >>>> explicitly added calcite to the jar, but not calcite's dependencies. I've >>>> been told this is the desired behavior as we want to ensure everything >>>> included is relocated. >>>> >>>> I don't know much about gradle, but this seems fragile. Is it possible >>>> to have all dependencies automatically relocated so we don't need the >>>> exclude(".*") rule? >>>> >>>> Andrew >>>> >>>> On Thu, May 17, 2018 at 7:41 PM Andrew Pilloud <apill...@google.com> >>>> wrote: >>>> >>>>> Yep, I added the issue as a blocker. >>>>> https://issues.apache.org/jira/projects/BEAM/issues/BEAM-4357 >>>>> >>>>> On Thu, May 17, 2018, 6:05 PM Kenneth Knowles <k...@google.com> wrote: >>>>> >>>>>> This sounds like a release blocker. Can you add it to the list? >>>>>> (Assign fix version on jira) >>>>>> >>>>>> Kenn >>>>>> >>>>>> On Thu, May 17, 2018, 17:30 Lukasz Cwik <lc...@google.com> wrote: >>>>>> >>>>>>> Typically we have a test block which uses a configuration that has >>>>>>> the shadow/shadowTest configurations on the classpath instead of the >>>>>>> compile/testCompile configurations. The most common examples are >>>>>>> validates >>>>>>> runner/integration tests for example: >>>>>>> >>>>>>> https://github.com/apache/beam/blob/0c5ebc449554a02cae5e4fd01afb07ecdb0bbaea/runners/direct-java/build.gradle#L84 >>>>>>> >>>>>>> On Thu, May 17, 2018 at 3:59 PM Andrew Pilloud <apill...@google.com> >>>>>>> wrote: >>>>>>> >>>>>>>> I decided to try our new JDBC support with sqlline and discovered >>>>>>>> that our SQL shaded jar is completely broken. As >>>>>>>> in java.lang.NoClassDefFoundError all over the place. How are we >>>>>>>> testing >>>>>>>> the output jars from other beam packages? Is there an example I can >>>>>>>> follow >>>>>>>> to make our integration tests run against the release artifacts? >>>>>>>> >>>>>>>> Andrew >>>>>>>> >>>>>>>
