[Result]
This vote close with two +1(one binding, one non binding), one 0(non
binding), one -1(non binding).
Thanks for all you guys feedback~!
JongWook reported two issues.
1. logging facade.
I prefer exclude all sl4j then use logback for now, because it is easy.
I am not saying we should stick to logback, but since there was no
discussion on logging facade, I think we open discuss after this release,
then fix whatever we decide.
Because of this issue, our package success, but binary actually not run
with example correctly, so I think this is blocker.
If others agree, then I will create RC6 after adding following on build.sbt.
# s2core/build.sbt
```
.map { moduleId =>
moduleId.exclude("org.mortbay.jetty", "*").exclude("javax.servlet", "*")
.exclude("org.slf4j", "slf4j-log4j12").exclude("org.slf4j",
"slf4j-jdk14")
}
```
2. loader test failure.
I forgot to mention that test case fail only because test case code did not
apply changes on s2core. `sbt "project loader" assembly` create working jar
that can be used for bulk upload.
However, since we change our default metastore from mysql to h2 embedded
mode, test cases on subproject other than s2core and s2rest_play need major
overhaul.
Current test case codes expect same metastore database with s2core and
s2rest_play.
I think we should either change embedded h2 to server mode so multiple
subproject use same metastore or change test cases code not to expect same
metastore between other subproject.
In my opinion, failed test case is not actually testing logic so would be
fine to comment out it before we fix test case environment. Also change
root project aggregate other subproject such as loader, s2counter_core,
s2counter_loader need to be done after setting up metastore issue. I think
this needs quite time to resolve so I prefer comment out failed test cases,
then re-write test cases on next version.
BTW, I have opened discussion thread suggesting re-write s2counter_core,
s2counter_loader project few month ago, but there was no follow up issue or
commits. Because of there is no commits after initial code import, I think
we should work on these subproject on next version which means test cases
should be changed while src code change.
For being more productive, please give feedback on above two issues.
On Fri, Oct 7, 2016 at 5:58 PM daewon <[email protected]> wrote:
> -1 (non-binding)
>
> From hsleep's advice
> I added lines below for exclude sl4j and it works.
>
> but I don't know what is right way to solve this issue.
>
> # s2core/build.sbt
> ```
> .map { moduleId =>
> moduleId.exclude("org.mortbay.jetty", "*").exclude("javax.servlet", "*")
> .exclude("org.slf4j", "slf4j-log4j12").exclude("org.slf4j",
> "slf4j-jdk14")
> }
> ```
> # s2core/build.sbt
> libraryDependencies ++= Seq(
> "ch.qos.logback" % "logback-classic" % "1.1.2",
> "com.typesafe" % "config" % "1.2.1",
> "com.typesafe.play" %% "play-json" % Common.playVersion,
> "com.typesafe.akka" %% "akka-actor" % "2.3.4",
> "com.typesafe.akka" %% "akka-slf4j" % "2.3.4",
> "org.apache.hbase" % "hbase-client" % Common.hbaseVersion,
> "org.apache.hbase" % "hbase-common" % Common.hbaseVersion,
> "org.apache.hbase" % "hbase-server" % Common.hbaseVersion,
> "org.apache.hadoop" % "hadoop-common" % Common.hadoopVersion,
> "org.apache.hadoop" % "hadoop-hdfs" % Common.hadoopVersion,
> "commons-pool" % "commons-pool" % "1.6",
> "org.scalatest" %% "scalatest" % "2.2.4" % "test",
> "org.scalikejdbc" %% "scalikejdbc" % "2.1.+",
> "mysql" % "mysql-connector-java" % "5.1.28",
> "org.apache.kafka" % "kafka-clients" % "0.8.2.1",
> "com.github.danielwegener" % "logback-kafka-appender" % "0.0.4",
> "org.apache.tinkerpop" % "gremlin-core" % "3.2.1",
> "org.apache.tinkerpop" % "tinkergraph-gremlin" % "3.2.1",
> "org.apache.tinkerpop" % "gremlin-groovy" % "3.2.1",
> "org.apache.tinkerpop" % "gremlin-test" % "3.2.1",
> "commons-cli" % "commons-cli" % "1.3.1"
> ).map { moduleId =>
> moduleId.exclude("org.mortbay.jetty", "*").exclude("javax.servlet", "*")
> .exclude("org.slf4j", "slf4j-log4j12").exclude("org.slf4j",
> "slf4j-jdk14")
> }
>
>
> On Fri, Oct 7, 2016 at 5:01 AM Jong Wook Kim <[email protected]> wrote:
>
> > Let me cast a
> >
> > +0 (non-binding)
> >
> >
> > 1. we messed up the logging facade.
> >
> > slf4j-log4j12.jar and log4j-over-slf4j.jar being present in the same
> > classpath is a serious problem, and we also have logback-classic with
> this.
> >
> > For anyone not familiar with how SLF4j bridges work, I encourage to refer
> > to: http://slf4j.org/legacy.html <http://slf4j.org/legacy.html>
> >
> > It is ideal for us to use only one of log4j or logback, but we are using
> > the both. (there are 2 logback.xml and 3 log4j.properties in the tree)
> >
> > I suppose that using log4j should be better for us, not because logback’s
> > functionality is worse but because that’s what other Apache projects are
> > doing.
> >
> > Currently asynchbase and playframework are the two non-Apache depenencies
> > that pulls logback (it’s stupid that they have it as compile dependency),
> >
> > and we need to carefully exclude logback from each of our subprojects and
> > move logback settings xml to log4j.properties.
> >
> > We might still have a valid reason to logback in our play apps, because
> > Play! directly uses some of logback’s API besides slf4j.
> >
> > In that cases we could diverge from other Apache projects and use Logback
> > altogether, but the important thing is to use only one of those and not
> to
> > mix up the SLF4j bridges.
> >
> >
> > 2. sbt “project loader” test:compile still fails, and it’s not caught
> > during sbt test because the loader project is not included in the main
> > aggregate.
> >
> >
> > I guess neither of these are critical for the 0.1.0 release, but I would
> > like to know what others think.
> >
> >
> > JW
> >
> > > On Oct 5, 2016, at 10:47 AM, daewon <[email protected]> wrote:
> > >
> > > -1 (non-binding)
> > >
> > > i tried rc5 and it fail.
> > >
> > > Evn info
> > > ```
> > > openjdk version "1.8.0_91"
> > > OpenJDK Runtime Environment (build
> > 1.8.0_91-8u91-b14-0ubuntu4~16.04.1-b14)
> > > OpenJDK 64-Bit Server VM (build 25.91-b14, mixed mode)
> > >
> > > ubuntu/stretch/sid
> > > ```
> > >
> > >
> > >
> > > root@91720ba55598
> >
> :/apps/apache-s2graph-0.1.0-incubating-src/target/apache-s2graph-0.1.0-incubating-bin/logs#
> > > tail -f s2rest_play.log
> > > at
> > play.core.StaticApplication.<init>(ApplicationProvider.scala:53)
> > > at
> > play.core.server.NettyServer$.createServer(NettyServer.scala:253)
> > > at
> > >
> play.core.server.NettyServer$$anonfun$main$3.apply(NettyServer.scala:289)
> > > at
> > >
> play.core.server.NettyServer$$anonfun$main$3.apply(NettyServer.scala:284)
> > > at scala.Option.map(Option.scala:146)
> > > at play.core.server.NettyServer$.main(NettyServer.scala:284)
> > > at play.core.server.NettyServer.main(NettyServer.scala)
> > > Caused by: java.lang.IllegalStateException: Detected both
> > > log4j-over-slf4j.jar AND slf4j-log4j12.jar on the class path,
> preempting
> > > StackOverflowError. See also
> > > http://www.slf4j.org/codes.html#log4jDelegationLoop for more details.
> > > at
> > >
> org.apache.log4j.Log4jLoggerFactory.<clinit>(Log4jLoggerFactory.java:51)
> > > ... 14 more
> > >
> > >
> > > On Tue, Oct 4, 2016 at 2:11 PM DO YUNG YOON <[email protected]> wrote:
> > >
> > >> +1 (non-binding)
> > >>
> > >> I tried RC5 on ubuntu 16.04, SBT 0.13.9, SCALA 2.11.7, OpenJDK
> 1.8.0_91.
> > >>
> > >> Checked followings.
> > >>
> > >> 1. Checksums and PGP signatures are valid
> > >> 2. Release consists of source code only, no binaries
> > >> 3. DISCLAIMER is correct, filenames include "incubating", top level
> > NOTICE
> > >> and LICENSE files.
> > >> 4. Check license header on files.
> > >> 5. Test if I can run `bin/example.sh` after build.
> > >>
> > >> Thanks.
> > >>
> > >> On Tue, Oct 4, 2016 at 2:13 PM DO YUNG YOON <[email protected]> wrote:
> > >>
> > >>> Hi all
> > >>>
> > >>> This is a call for a releasing Apache S2Graph 0.1.0-incubating,
> release
> > >>> candidate 5.
> > >>> This is the first release of S2Graph.
> > >>>
> > >>> The source tarball, including signatures, digests, etc. can be found
> > at:
> > >>>
> > >>>
> > >>
> >
> https://dist.apache.org/repos/dist/dev/incubator/s2graph/0.1.0-incubating-RC5/
> > >>>
> > >>>
> > >>> The tag to be voted upon is v0.1.0-incubating-rc5:
> > >>>
> > >>>
> > >>
> >
> https://git-wip-us.apache.org/repos/asf?p=incubator-s2graph.git;a=shortlog;h=refs/tags/v0.1.0-incubating-rc5
> > >>>
> > >>> The release hash is 4b6ffc2e357afc005b194d6d45710c2b041b00cb:
> > >>>
> > >>>
> > >>
> >
> https://git-wip-us.apache.org/repos/asf?p=incubator-s2graph.git;a=commit;h=4b6ffc2e357afc005b194d6d45710c2b041b00cb
> > >>>
> > >>>
> > >>> Release artifacts are signed with the following key:
> > >>> https://dist.apache.org/repos/dist/dev/incubator/s2graph/KEYS
> > >>>
> > >>> Once download source, please look into README.md to build from
> source.
> > >>>
> > >>> Notable changes from RC4 is removing custom repo for asynchbase
> > >> dependency(
> > >>> S2GRAPH-116 <https://github.com/apache/incubator-s2graph/pull/85>).
> > >>>
> > >>> The vote will be open for at least 72 hours. Unless objection I will
> > try
> > >>> to close it Sunday September 25 if we have sufficient votes.
> > >>>
> > >>> Please download the release candidate and evaluate the necessary
> items
> > >>> including checking hashes, signatures, build from source, and test.
> > >>> please vote:
> > >>>
> > >>> [ ] +1 Release this package as 0.1.0
> > >>> [ ] +0 no opinion
> > >>> [ ] -1 Do not release this package because...
> > >>>
> > >>> Thanks,
> > >>> DOYUNG YOON.
> > >>>
> > >>>
> > >>
> >
> >
>
