I created a pull request with #2 implemented. Any comments would be appreciated. https://github.com/apache/incubator-rya/pull/101
On Fri, Oct 7, 2016 at 11:06 AM, Puja Valiyil <[email protected]> wrote: > Hey, > I started trying to implement #2. Give me 10 minutes and I'll push it and > you can start from there. > Sorry I should have communicated that earlier in this thread. > > On Fri, Oct 7, 2016 at 11:01 AM, Aaron D. Mihalik <[email protected] > > wrote: > >> Okay, I'm going to take another shot at the "profile" solution and remove >> tinkerpop.rya. I'll post the PR to the dev list and give let people >> comment on the PR. I'll look at PR over the weekend if if there aren't >> any >> issues, I'll pull it into apache master on Sunday. >> >> --Aaron >> >> On Fri, Oct 7, 2016 at 10:42 AM Puja Valiyil <[email protected]> wrote: >> >> > I'm fine with that. >> > >> > On Fri, Oct 7, 2016 at 9:29 AM, Aaron D. Mihalik < >> [email protected]> >> > wrote: >> > >> > > Can we remove tinkerpop.rya? I thought that this was part of the >> query >> > > based reasoning, but the inference engine / rya.sail does not have a >> > > dependency on rinkerpop.rya. >> > > >> > > --Aaron >> > > >> > > >> > > On Fri, Oct 7, 2016 at 7:39 AM Puja Valiyil <[email protected]> >> wrote: >> > > >> > > The reasoning here is not the query based inference-- it is the >> external >> > > reasoner that runs on map reduce. >> > > I need to double check but I think the dependency is due to >> referencing a >> > > config Utilities class that should be in accumulo Rya not in >> indexer. So >> > > moving a single class to a different project will likely fix a lot of >> > this. >> > > >> > > Sent from my iPhone >> > > >> > > > On Oct 6, 2016, at 10:16 PM, Aaron D. Mihalik < >> [email protected] >> > > >> > > wrote: >> > > > >> > > > After reviewing the PR that David submitted, it's concerning the >> number >> > > of >> > > > projects that would fall into this "optional" bin. Some users >> probably >> > > > consider these "core" functions (e.g. reasoning and web): >> > > > >> > > > Here the modules that need to be removed from the build in order to >> > > remove >> > > > the geotools references: >> > > > >> > > > mapreduce >> > > > indexing >> > > > rya.indexing.pcj >> > > > indexingExample >> > > > rya.pcj.fluo >> > > > tinkerpop.rya >> > > > web.rya >> > > > rya.reasoning >> > > > rya.console >> > > > rya.merger >> > > > >> > > > --Aaron >> > > > >> > > >> On Thu, Oct 6, 2016 at 3:41 PM David Lotts <[email protected]> >> wrote: >> > > >> >> > > >> Yes, geotools is a runtime dependency. No geotools source code is >> > > >> distributed. >> > > >> >> > > >> By that I mean: Geotools source code is not in our source code >> > > repository. >> > > >> Only references: imports in our *.java files and dependencies >> entries >> > in >> > > >> our pom.xml. Because of this maven will package geotools JARs >> > > (binaries) >> > > >> in our shaded/uber JAR and WAR files that we distribute. >> > > >> >> > > >> With option 1 or 2 as discussed, maven will exclude the geotools >> jars >> > in >> > > >> our JARs and WARs. Users of Rya can follow some instructions that >> we >> > > >> provide to add "-P indexing" (or similar) to their Maven build >> command >> > > >> create their own jar/war containing the optional Rya features and >> > > geotools >> > > >> binaries. >> > > >> >> > > >> Your "you should be okay." mean which of these???? >> > > >> A. option 1 and option 2 will work around the issue and we should >> > > proceed >> > > >> before we release, >> > > >> - OR - >> > > >> B. We are already in compliance and this is not a blocker for >> release >> > > as >> > > >> long as we are not redistributing geotools source code. >> > > >> >> > > >> Hopeful for interpretation B, but expecting and happy with A. >> > > >> >> > > >> david. >> > > >> >> > > >> On Thu, Oct 6, 2016 at 1:22 PM, Seetharam Venkatesh < >> > > [email protected] >> > > > >> > > >> wrote: >> > > >> >> > > >>> Quick question - geotools is a runtime dependency? Are you >> shipping >> > the >> > > >>> source code? If not, you should be okay. >> > > >>> >> > > >>> Sent from my iPhone, >> > > >>> Venkatesh >> > > >>> >> > > >>>> On Oct 6, 2016, at 7:52 AM, Puja Valiyil <[email protected]> >> wrote: >> > > >>>> >> > > >>>> Hi everyone, >> > > >>>> Talking with Aaron, it seems like there were two paths forward >> for >> > > >>>> refactoring in order to create a release. To refresh everyone's >> > > >> memory, >> > > >>>> the issue was that the geo-indexing extensions to Rya pull in >> > > geotools, >> > > >>>> which prohibits us from releasing Rya under an Apache 2 license. >> > > There >> > > >>> may >> > > >>>> be some more particulars that I'm glossing over -- someone please >> > > chime >> > > >>> in >> > > >>>> if they feel it is key to the discussion. >> > > >>>> The two paths forward we had were: >> > > >>>> 1. Make all of the indexing project and its downstream >> dependencies >> > > >>>> optional and exclude them from a release >> > > >>>> -- The indexing project includes several "optional" extensions to >> > Rya >> > > >>>> (advanced indexing strategies). Prior to Rya becoming an apache >> > > >> project, >> > > >>>> these indexing extensions were optional and there was a separate >> > > >> profile >> > > >>>> for including them. This option involves reverting back to that >> > > >> mindset. >> > > >>>> The main argument against this is that these indexing >> > > >>> strategies/extensions >> > > >>>> are not in fact optional but are "core" to Rya and can't be >> > excluded. >> > > >>>> >> > > >>>> 2. Refactor Rya to pull geoindexing into a separate project and >> > > >> exclude >> > > >>>> that project from the release. >> > > >>>> - We could refactor Rya to have geoindexing be its own project >> and >> > add >> > > >> a >> > > >>>> profile to include that in the build. This would invovle moving >> the >> > > >>> class >> > > >>>> mvm.rya.indexing.GeoIndexer and packages >> > mem.rya.indexing.accumulo.geo >> > > >>> and >> > > >>>> mvm.rya.indexing.mongodb.geo to a separate project and then >> > > >>> removing/moving >> > > >>>> references to geoindexing anywhere else. Another option is to >> > > refactor >> > > >>> the >> > > >>>> GeoIndexer interface to remove the geotools dependency. >> > > >>>> >> > > >>>> I think #1 is a good immediate path for a release and that #2 is >> a >> > > good >> > > >>>> longer term path forward. Since it's probably in our best >> interests >> > > >> as a >> > > >>>> community to get an apache release sooner rather than later, I'd >> > > rather >> > > >>> us >> > > >>>> go with #1 since it would quicker. I also think that most users >> of >> > > Rya >> > > >>>> would be ok with excluding the indexing project since it is not >> core >> > > >>>> functionality for Rya. While #2 is a better long term plan, it >> > > >> involves >> > > >>>> some pretty extensive refactoring that would be difficult to do >> well >> > > >> in a >> > > >>>> timely manner. >> > > >>>> >> > > >>>> Any thoughts? >> > > >> >> > > >> > >> > >
