Yup. FYI, this was Dave's comment about the Heron API from the RC vote discussion: "Is there a reason the source contains Apache Commons CLI and Lang3 source code?"
That's why I was leaning towards publishing the original `java_library` and not the incorrectly shaded build target. And the pom.xml would include the dependencies needed. Sounds like we're in agreement. If I don't see a PR, I'll soon have cycles to take a look at this again. Ended up running into some other issues that I had to resolve first (Helm chart bug #3775, Heron UI bug #3777) On Fri, Feb 25, 2022 at 10:33 PM Josh Fischer <[email protected]> wrote: > Attempted answers below: > > 1. Do we want to keep shading dependencies, or should we include the > dependencies in the pom.xml? > - From what I can tell, the shading is only configured to explicitly work > with the protobuf package. I don't see any conflicting versions in this > package, so I'm not sure what the reason would be to shade with the > exception of including al the transitive dependencies into a fat jar. ( I > think that is how it shading works). *(a.)* > 2. What is classification jar? > - This is an implementation of a javax annotation processor. It's a > developmen tool to help document the code on how and who would be using > interfaces in the Heron api. *(b.)* > 3. We are publishing a Heron API jar (that final artifact), but our > examples are referencing the more immediate streamlet or low-level jars. > Should the examples reference that final jar? > - I think the examples should reference the final artifact jar. > > > a. > > https://github.com/apache/incubator-heron/blob/master/heron/api/src/java/shade.conf > b. > > https://github.com/apache/incubator-heron/tree/master/heron/api/src/java/org/apache/heron/classification > > > > > On Wed, Feb 23, 2022 at 6:45 PM Josh Fischer <[email protected]> wrote: > > > Ok, what I understand is that we need to fix this shading issue by hard > > coding dependencies into the pom for a short term win before creating the > > next RC. Long term we can look into this article > > > https://medium.com/wix-engineering/how-to-publish-artifacts-from-bazel-to-maven-central-71da0cf990f5 > for > > removing the hard coded deps in the pom file. If this is the case, no > need > > to reply. If not, please let me know before I start to look into it. > > > > On Wed, Feb 23, 2022 at 6:10 PM Nicholas Nezis <[email protected] > > > > wrote: > > > >> Longer term we should look to this Bazel plugin to manage the Maven > style > >> build artifacts. We don't have to use it for publishing, but at least > help > >> us generate the jar and matching pom.xml with proper dependencies. I > don't > >> like having hard coded dependencies in a random pom.xml template. This > >> tool > >> seems like it could help with that. > >> > >> > https://medium.com/wix-engineering/how-to-publish-artifacts-from-bazel-to-maven-central-71da0cf990f5 > >> > >> But this is work to be done after the RC. > >> > >> On Wed, Feb 23, 2022 at 7:08 PM Nicholas Nezis < > [email protected]> > >> wrote: > >> > >> > Well the issue raised was us having dependencies incorrectly shaded. > So > >> > this is one potential solution to that issue. We could decide to keep > >> the > >> > shading and just fix that if it is sufficient to do that. That might > be > >> > easier. But typically projects will provide the unshaded jar with > >> > dependencies listed. In addition to Dave commenting on the weird > >> shading, I > >> > also had a team member of mine comment on the weirdness that Heron was > >> > providing the shaded jar. But those were the two main motivators > raising > >> > the issue. > >> > > >> > On Wed, Feb 23, 2022 at 6:45 PM Josh Fischer <[email protected]> > >> wrote: > >> > > >> >> Where did we decide we needed to change these poms? Was there an > issue > >> >> raised by one of the mentors? > >> >> > >> >> On Wed, Feb 23, 2022 at 4:06 PM Nicholas Nezis < > >> [email protected]> > >> >> wrote: > >> >> > >> >> > Yup. And how those pom.xml files relate to the generated jar files. > >> One > >> >> of > >> >> > the issues was that we are shading dependencies into the jar. We > >> should > >> >> > instead be providing the jar with only our code, and the pom.xml > >> should > >> >> > include the dependencies. > >> >> > > >> >> > The scripts to generate the pom.xml files: > >> >> > https://github.com/apache/incubator-heron/tree/master/scripts/ci > >> >> > And the scripts to build the Heron API jar(s): > >> >> > > >> >> > > >> >> > >> > https://github.com/apache/incubator-heron/blob/abb2767e3df3ca6eba009f46efe1f1e83695617a/heron/api/src/java/BUILD#L23-L87 > >> >> > > >> >> > "api-java-low-level": Normal "Topology API" jar > >> >> > "api-java": Functional (i.e. Streamlet jar) > >> >> > "api-java-low-level-functional": Combination of topology and > >> streamlet > >> >> code > >> >> > "api-unshaded": Java Binary (why a binary???) with both, but kryo > >> >> neverlink > >> >> > dependency added. I think this might be for Storm compatibility? > >> >> > "api-shaded": Remaps the protobuf classes based on this rule... but > >> the > >> >> > rule doesn't shade any of the other dependencies... If we will > >> continue > >> >> > shading, we should fix this. > >> >> > > >> >> > > >> >> > >> > https://github.com/apache/incubator-heron/blob/master/heron/api/src/java/shade.conf > >> >> > "heron-api": We create a copy of the heron-api.jar for some > >> reason... no > >> >> > idea why this is this way and we don't just rename the previouis > >> build > >> >> task > >> >> > "classification": No idea what this is... is it part of the Heron > >> API? > >> >> Why > >> >> > is it not included in the resulting "heron-api" jar? > >> >> > > >> >> > Questions I have: > >> >> > 1. Do we want to keep shading dependencies, or should we include > the > >> >> > dependencies in the pom.xml? > >> >> > 2. What is classification jar? > >> >> > 3. We are publishing a Heron API jar (that final artifact), but our > >> >> > examples are referencing the more immediate streamlet or low-level > >> jars. > >> >> > Should the examples reference that final jar? > >> >> > > >> >> > I've tried to be diligent in studying the code to make sure I don't > >> >> break > >> >> > anything. Perhaps Ning can suggest how to test I didn't break Storm > >> >> > compatibility if I make changes. I don't know how to easily test > >> that. > >> >> Is > >> >> > it covered in the CI testing? > >> >> > > >> >> > I've been super busy lately, so haven't gotten to make progress on > >> this. > >> >> > But I would love help if someone has the cycles. > >> >> > > >> >> > On Wed, Feb 23, 2022 at 11:22 AM Josh Fischer <[email protected] > > > >> >> wrote: > >> >> > > >> >> > > In relation to > >> >> > > > https://github.com/apache/incubator-heron/projects/7#card-76168234 > >> >> > > > >> >> > > Are these the poms you are talking about? > >> >> > > > >> >> > > > >> https://github.com/apache/incubator-heron/tree/master/release/maven > >> >> > > > >> >> > > On Sat, Feb 12, 2022 at 3:24 PM Nicholas Nezis < > >> >> [email protected] > >> >> > > > >> >> > > wrote: > >> >> > > > >> >> > > > I've been tracking the items here: > >> >> > > > https://github.com/apache/incubator-heron/projects/7 > >> >> > > > > >> >> > > > I've been busy with work and life so haven't gotten to really > do > >> >> what I > >> >> > > > wanted with the cleanup of the API jars. Not sure how best to > >> test > >> >> my > >> >> > > > changes with Storm compatibility. I also started messing around > >> >> with a > >> >> > > new > >> >> > > > Bazel plugin that generates the Pom files, but I think I'm > going > >> to > >> >> > table > >> >> > > > that for now. Will just update the existing pom.xml templates > we > >> >> have. > >> >> > My > >> >> > > > testing will be limited to submitting the samples to verify > that > >> >> they > >> >> > > still > >> >> > > > work. Any help testing my changes would be appreciated. I'll > try > >> to > >> >> get > >> >> > > > something that I can check in for review. > >> >> > > > > >> >> > > > On Sat, Feb 12, 2022 at 4:19 PM Josh Fischer < > >> [email protected]> > >> >> > > wrote: > >> >> > > > > >> >> > > > > Does anything else need to be done before we cut another > rc?-- > >> >> > > > > Sent from A Mobile Device > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> >> > >> > > >> > > >
