Hi JongWook. Thanks for the detailed feedback. Here is what I think about for the issues.
1. StrongLabelDeleteTest.deleteAll fails randomly I have experienced this before with following conditions. 1. run `sbt test` 2. remove metastore folder. 3. run `sbt test` We are relying on Metastore's sequence for each label, service etc so removing metastore folder would result in same sequence id and this yield test affected by previous `sbt test`( https://issues.apache.org/jira/browse/S2GRAPH-101 for more detail). Also I agree that we should comment that test require HBase setup, or we should move on making our test to run with embedded hbase so each test run will clean up previous test data. Even though I think this is critical issue, I don't think this is blocker for our first release. We can always release next version focusing on change our test cases and so on. I am wondering what others think about this. 2. asynchbase dependency in the POM Like you point out, users who want to download S2Graph as dependency from Maven central will fail. During incubating process, we only release our source code, not binary, so I don't think it is a blocker(Please correct me if I am wrong). Of course It would be great to find out work around for this issue, but since my lack of knowledge and experience, I have no idea how to resolve this except sending PR to asynchbase upstream and hope PR get merged and next release of asynchbase contains it. 3. SBT root project depends on others but only aggregates s2core and s2rest_play Agree, and I am going to create issue, but also don't think it block our first release. 4. Travis CI fails to resolve SBT dependencies I checked https://travis-ci.org/jongwook/incubator-s2graph/builds/162882670, but can't figure out what is problem. I don't think these issues are not blocker(not sure about 2 though). Folks, Please comment what you guys think about these issues and RC4. Thanks. On Tue, Sep 27, 2016 at 11:54 AM Jong Wook Kim <[email protected]> wrote: > a correction - I meant it should be better to stick with Maven Central, > not adding a custom one. > > JW > > > > On Sep 26, 2016, at 4:15 PM, Jong Wook Kim <[email protected]> wrote: > > > > I’m not sure how critical or trivial these are, but I’d like to share > some issues: > > > > > > > > 1. StrongLabelDeleteTest.deleteAll fails randomly > > > > First of all I have one test case that is failing, with the following > log. > > > > [info] StrongLabelDeleteTest: > > [info] - Strong consistency select (274 milliseconds) > > [info] - Strong consistency deleteAll (83 milliseconds) > > [info] - update delete (10 seconds, 377 milliseconds) > > [info] - update delete 2 (7 seconds, 611 milliseconds) > > [info] - large degrees (83 milliseconds) > > [info] - deleteAll *** FAILED *** (94 milliseconds) > > [info] false was not true (StrongLabelDeleteTest.scala:211) > > [info] org.scalatest.exceptions.TestFailedException: > > [info] at > org.scalatest.MatchersHelper$.newTestFailedException(MatchersHelper.scala:160) > > [info] at > org.scalatest.Matchers$ShouldMethodHelper$.shouldMatcher(Matchers.scala:6231) > > [info] at > org.scalatest.Matchers$AnyShouldWrapper.should(Matchers.scala:6265) > > [info] at > org.apache.s2graph.core.Integrate.StrongLabelDeleteTest$$anonfun$9.apply$mcV$sp(StrongLabelDeleteTest.scala:211) > > [info] at > org.apache.s2graph.core.Integrate.StrongLabelDeleteTest$$anonfun$9.apply(StrongLabelDeleteTest.scala:181) > > [info] at > org.apache.s2graph.core.Integrate.StrongLabelDeleteTest$$anonfun$9.apply(StrongLabelDeleteTest.scala:181) > > [info] at > org.scalatest.Transformer$$anonfun$apply$1.apply$mcV$sp(Transformer.scala:22) > > > > This is not entirely deterministic, as it sometimes succeeds, but most > of the time it fails with the stacktrace above. > > > > If the test is dependent on some preconditions I think it’d be good to > include the initialization code that makes the precondition true. > > > > The documentation lacks the comment that HBase must be running during > the test; maybe better to launch/stop HBase before/after the test? > > > > > > > > 2. asynchbase dependency in the POM > > > > After running “sbt publishM2”, the resulting POM has the following entry > as a dependency, > > > > <dependency> > > <groupId>org.hbase</groupId> > > <artifactId>asynchbase</artifactId> > > <version>1.7.2-S2GRAPH</version> > > </dependency> > > > > but the resolution of any Maven project dependent on s2core fails > because the POM doesn’t contain > https://github.com/SteamShon/asynchbase/raw/mvn-repo < > https://github.com/SteamShon/asynchbase/raw/mvn-repo> as an additional > Maven repository. > > > > This can be fixed by manually adding the url as an additional Maven > resolver in the SBT settings, but generally it is not a good idea to use > the Maven central only rather than adding a custom repo. > > > > This might not be a problem if we primarily focus on the S2Graph server > application in this release, but if we are uploading the S2Graph artifacts > to the Maven Central, e.g. to support the custom batch job development, > this needs to be fixed. > > > > > > > > > > 3. SBT root project depends on others but only aggregates s2core and > s2rest_play > > > > Currently the root SBT project only aggregates s2core and s2rest_play, > and thus any sbt command in the root directory, like compile, test, and > publish, will be only applied to those two. > > > > > > > > > > 4. Travis CI fails to resolve SBT dependencies > > > > This is most likely a Travis CI issue; it fails while resolving some SBT > plugins: https://travis-ci.org/jongwook/incubator-s2graph/builds < > https://travis-ci.org/jongwook/incubator-s2graph/builds> > > > > > > > > > > Jong Wook > > > > > > > >> On Sep 26, 2016, at 9:27 AM, DO YUNG YOON <[email protected] <mailto: > [email protected]>> wrote: > >> > >> Hi all. > >> > >> So far, we got 1(+1 binding) and 3(+1 non-binding). > >> Thanks for taking time to try out RC4. > >> But I think we need more +1. > >> > >> Please take time to try out our RC4 then give any feedback and vote. > >> I will wait more days before closing out this vote thread until either > >> objection or sufficient votes. > >> > >> > >> > >> > >> On Fri, Sep 23, 2016 at 3:00 PM Sergio Fernández <[email protected] > <mailto:[email protected]>> wrote: > >> > >>> On Fri, Sep 23, 2016 at 12:51 AM, Jong Wook Kim <[email protected] > <mailto:[email protected]>> wrote: > >>> > >>>> I wrote that section in the README.md in mind that the majority will > >>>> download the binary distribution which will contain the content in the > >>>> subdirectory, > >>>> > >>>> The section “Building from the source” specifies that the distribution > >>>> will be created in target/apache-s2graph-$version-incubating-bin. > >>>> > >>>> So I don’t think this is a blocker, but I agree that README.md can be > >>> even > >>>> more specific and it would be nice if start-s2graph.sh prints a > helpful > >>>> message if it is launched in the source root. > >>>> > >>> > >>> No no, the mistake was completely on my side, I just wanted to provide > >>> feedback from outside of the project development. Of course this is not > >>> blocking the release, it's just a comment. > >>> > >>> > >>> > >>> > >>> > >>>>> On Sep 22, 2016, at 6:02 AM, Sergio Fernández <[email protected] > <mailto:[email protected]>> > >>> wrote: > >>>>> > >>>>> On Thu, Sep 22, 2016 at 11:56 AM, Kim, Min-Seok <[email protected] > <mailto:[email protected]>> > >>>> wrote: > >>>>>> > >>>>>> To Sergio, > >>>>>> > >>>>>> The commands will work on > >>>>>> > >>> `/path/to/incubator-s2graph/target/apache-s2graph-0.1.0-incubating-bin` > >>>>>> after `sbt package`. > >>>>>> > >>>>> > >>>>> Of course! > >>>>> > >>>>> Then the README needs to remark that to avoid people to get confused, > >>>> like > >>>>> it happened to me. > >>>>> > >>>>> Thanks! > >>>>> > >>>>> -- > >>>>> Sergio Fernández > >>>>> Partner Technology Manager > >>>>> Redlink GmbH > >>>>> m: +43 6602747925 > >>>>> e: [email protected] <mailto:[email protected]> > >>>>> w: http://redlink.co <http://redlink.co/> > >>>> > >>>> > >>> > >>> > >>> -- > >>> Sergio Fernández > >>> Partner Technology Manager > >>> Redlink GmbH > >>> m: +43 6602747925 > >>> e: [email protected] <mailto:[email protected]> > >>> w: http://redlink.co <http://redlink.co/> > >>> > > > >
