Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Scrooge3upgrade #22

Merged
merged 6 commits into from

4 participants

@jowens
Collaborator

I believe these changes work properly with scrooge3. They upgrade a significant number of library versions, though, and I'm also not well-versed in either a) who else uses this library and might face the ramifications or b) scrooge3 and what else might break beyond the simple searchbird example that I've used. So actually looking at my changes would be appreciated.

Franklin Hu, on my changes: "Yeah that looks pretty reasonable."

@stevegury stevegury commented on the diff
lib/template/project/plugins.sbt
((6 lines not shown))
-addSbtPlugin("com.twitter" %% "sbt11-scrooge" % "1.0.0")
+addSbtPlugin("com.twitter" %% "sbt11-scrooge" % "3.0.0")
+
+addSbtPlugin("com.twitter" % "sbt-thrift2" % "0.0.1")

Because of some previous ABI incompatibility, the scala convention is to happend the version of scala to the name of the jar. Sbt push this concept a little bit further by adding the version of sbt.
The people that developed those plugins have stopped maintaining them (maybe because there were tired of releasing a new version of the plugin for each new version of sbt). For those plugins the latest build available is for sbt 0.11.2, if you use another version of sbt this code won't compile.

A pragmatic way of avoiding this version number nightmare would be to provide your own sbt script like in finagle (See https://github.com/twitter/finagle/blob/master/sbt)

@jowens Collaborator
jowens added a note

@stevegury: OK, I'll need some elaboration on this. :) Adding my own sbt script would a) do what in particular and b) obviate the need for which plugins? Does this seem to you to be the right thing to do?

I'm trying to do a balance between "do this right" and "not want to spend 3 weeks writing shell scripts".

Here you don't specify the sbt version the user has to use, so depending on which version of sbt do you have this code will potentially fail.
On my machine I have sbt 0.11.3-2, if I load this project, it will fail because it can't find sbt-package-dist_2.9.1_0.11.3 version 1.0.5

If you provide a script (just copy paste the one in finagle) that run a specific version of sbt (0.11.2), it will succeed because sbt-package-dist_2.9.1_0.11.2 version 1.0.5 is available.

@jowens Collaborator
jowens added a note

OK. I added this sbt script and changed the other files accordingly. Great suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/template/build.sbt.erb
@@ -17,16 +17,17 @@ version := "1.0.0-SNAPSHOT"
libraryDependencies ++= Seq(
"org.scala-lang" % "jline" % "2.9.1",
- "com.twitter" %% "scrooge-runtime" % "1.1.3",
- "com.twitter" %% "finagle-core" % "3.0.0",
- "com.twitter" %% "finagle-thrift" % "3.0.0",
- "com.twitter" %% "finagle-ostrich4" % "3.0.0",
- "org.scalatest" %% "scalatest" % "1.7.1" % "test",
- "com.twitter" %% "scalatest-mixins" % "1.0.3" % "test"
+ "com.twitter" % "scrooge" % "3.0.1" intransitive(),
+ "com.twitter" % "scrooge-runtime_2.9.2" % "3.0.1" intransitive(),

Why did you add "intransitive"?

@jowens Collaborator
jowens added a note

Franklin Hu: "You shouldn't have to make changes in the generated files, we just marked scrooge as intransitive() to prevent it from pulling in thrift 0.8.0." and later "You just have to make scrooge not pull in newer version of thrift by marking them intransitive since there have been API changes between 0.5.0 and 0.8.0." (That's what they're doing in the Zipkin build.)

@chunyan
chunyan added a note

Is it intentional to use THRIFT 0.5.0, not THRIFT 0.8.0? Scrooge 3 implements THIRFT 0.8.0, I believe.

@jowens Collaborator
jowens added a note

I don't know, but I'll ask Franklin to comment on this.

Just revisited and you may be okay without marking it intransitive.
We had to pin the dependency in Zipkin since cassie depends on finagle-thrift, which uses thrift 0.5.0. Cassie has a FakeCassandra that starts up a thrift TThreadPoolServer whose constructor signature has changed between 0.5.0 and 0.8.0.
Sorry for the confusion.

@jowens Collaborator
jowens added a note

OK, so delete the two "intransitive()" and everything else is the same?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@franklinhu franklinhu was assigned
@jowens
Collaborator

Oops! "Finagle". Doh!

@chunyan chunyan was assigned
@jowens
Collaborator

Only change is removal of intransitive(), everything else is whitespace

@chunyan chunyan commented on the diff
lib/template/build.sbt.erb
@@ -16,17 +16,18 @@ name := "birdname"
version := "1.0.0-SNAPSHOT"
libraryDependencies ++= Seq(
- "org.scala-lang" % "jline" % "2.9.1",
- "com.twitter" %% "scrooge-runtime" % "1.1.3",
- "com.twitter" %% "finagle-core" % "3.0.0",
- "com.twitter" %% "finagle-thrift" % "3.0.0",
- "com.twitter" %% "finagle-ostrich4" % "3.0.0",
- "org.scalatest" %% "scalatest" % "1.7.1" % "test",
- "com.twitter" %% "scalatest-mixins" % "1.0.3" % "test"
+ "org.scala-lang" % "jline" % "2.9.1",
+ "com.twitter" % "scrooge" % "3.0.1",
+ "com.twitter" % "scrooge-runtime_2.9.2" % "3.0.1",
+ "com.twitter" % "finagle-core" % "5.3.6",
@chunyan
chunyan added a note

Scrooge 3.0.1 depends on the following versions of util and finagle
finagle-core: 5.0.0
finagle-thirft: 5.0.0
finagle-ostrich4: 5.0.0
util-core: 5.0.3
util-codec: 5.0.3

Does it work if you put newer versions here? (it might, but I just want to make sure)

@jowens Collaborator
jowens added a note

It appears to work for the purposes of initializing searchbird. I just picked the latest versions of the three finagle libraries (and did not specify the util-* libraries). But of course searchbird is only one use instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chunyan chunyan merged commit e38ae6c into twitter:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
1  bin/scala-bootstrapper
@@ -125,6 +125,7 @@ end
"src/scripts/#{project_name.downcase}.sh",
"src/scripts/devel.sh",
"console",
+ "sbt",
].each do |executable|
`chmod +x #{executable}` if File.exists?(executable)
end
View
17 lib/template/build.sbt.erb
@@ -16,17 +16,18 @@ name := "birdname"
version := "1.0.0-SNAPSHOT"
libraryDependencies ++= Seq(
- "org.scala-lang" % "jline" % "2.9.1",
- "com.twitter" %% "scrooge-runtime" % "1.1.3",
- "com.twitter" %% "finagle-core" % "3.0.0",
- "com.twitter" %% "finagle-thrift" % "3.0.0",
- "com.twitter" %% "finagle-ostrich4" % "3.0.0",
- "org.scalatest" %% "scalatest" % "1.7.1" % "test",
- "com.twitter" %% "scalatest-mixins" % "1.0.3" % "test"
+ "org.scala-lang" % "jline" % "2.9.1",
+ "com.twitter" % "scrooge" % "3.0.1",
+ "com.twitter" % "scrooge-runtime_2.9.2" % "3.0.1",
+ "com.twitter" % "finagle-core" % "5.3.6",
@chunyan
chunyan added a note

Scrooge 3.0.1 depends on the following versions of util and finagle
finagle-core: 5.0.0
finagle-thirft: 5.0.0
finagle-ostrich4: 5.0.0
util-core: 5.0.3
util-codec: 5.0.3

Does it work if you put newer versions here? (it might, but I just want to make sure)

@jowens Collaborator
jowens added a note

It appears to work for the purposes of initializing searchbird. I just picked the latest versions of the three finagle libraries (and did not specify the util-* libraries). But of course searchbird is only one use instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ "com.twitter" % "finagle-thrift" % "5.3.6",
+ "com.twitter" % "finagle-ostrich4" % "5.3.1",
+ "org.scalatest" %% "scalatest" % "1.7.1" % "test",
+ "com.twitter" %% "scalatest-mixins" % "1.1.0" % "test"
)
mainClass in (Compile, run) := Some("com.twitter.birdname.Main")
mainClass in (Compile, packageBin) := Some("com.twitter.birdname.Main")
-CompileThriftScrooge.scroogeVersion := "2.5.4"
+CompileThriftScrooge.scroogeVersion := "3.0.1"
View
2  lib/template/console.erb
@@ -5,4 +5,4 @@ if [ $# -lt 2 ] ; then
exit 0
fi
-sbt "run-main com.twitter.birdname.BirdNameConsoleClient $1 $2"
+./sbt "run-main com.twitter.birdname.BirdNameConsoleClient $1 $2"
View
6 lib/template/project/plugins.sbt
@@ -19,8 +19,10 @@ resolvers <<= (resolvers) { r =>
externalResolvers <<= (resolvers) map identity
-addSbtPlugin("com.twitter" %% "sbt-package-dist" % "1.0.4")
+addSbtPlugin("com.twitter" %% "sbt-package-dist" % "1.0.5")
-addSbtPlugin("com.twitter" %% "sbt11-scrooge" % "1.0.0")
+addSbtPlugin("com.twitter" %% "sbt11-scrooge" % "3.0.0")
+
+addSbtPlugin("com.twitter" % "sbt-thrift2" % "0.0.1")

Because of some previous ABI incompatibility, the scala convention is to happend the version of scala to the name of the jar. Sbt push this concept a little bit further by adding the version of sbt.
The people that developed those plugins have stopped maintaining them (maybe because there were tired of releasing a new version of the plugin for each new version of sbt). For those plugins the latest build available is for sbt 0.11.2, if you use another version of sbt this code won't compile.

A pragmatic way of avoiding this version number nightmare would be to provide your own sbt script like in finagle (See https://github.com/twitter/finagle/blob/master/sbt)

@jowens Collaborator
jowens added a note

@stevegury: OK, I'll need some elaboration on this. :) Adding my own sbt script would a) do what in particular and b) obviate the need for which plugins? Does this seem to you to be the right thing to do?

I'm trying to do a balance between "do this right" and "not want to spend 3 weeks writing shell scripts".

Here you don't specify the sbt version the user has to use, so depending on which version of sbt do you have this code will potentially fail.
On my machine I have sbt 0.11.3-2, if I load this project, it will fail because it can't find sbt-package-dist_2.9.1_0.11.3 version 1.0.5

If you provide a script (just copy paste the one in finagle) that run a specific version of sbt (0.11.2), it will succeed because sbt-package-dist_2.9.1_0.11.2 version 1.0.5 is available.

@jowens Collaborator
jowens added a note

OK. I added this sbt script and changed the other files accordingly. Great suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
libraryDependencies += "com.twitter" %% "scalatest-mixins" % "1.0.3"
View
42 lib/template/sbt
@@ -0,0 +1,42 @@
+#!/bin/bash
+
+root=$(
+ cd $(dirname $(readlink $0 || echo $0))/..
+ /bin/pwd
+)
+
+sbtjar=sbt-launch.jar
+
+if [ ! -f $sbtjar ]; then
+ echo 'downloading '$sbtjar 1>&2
+ curl -O http://repo.typesafe.com/typesafe/ivy-releases/org.scala-tools.sbt/sbt-launch/0.11.2/$sbtjar
+fi
+
+test -f $sbtjar || exit 1
+
+sbtjar_md5=$(openssl md5 < $sbtjar|cut -f2 -d'='|awk '{print $1}')
+
+if [ "${sbtjar_md5}" != 2886cc391e38fa233b3e6c0ec9adfa1e ]; then
+ echo 'bad sbtjar!' 1>&2
+ exit 1
+fi
+
+test -f ~/.sbtconfig && . ~/.sbtconfig
+
+java -ea \
+ $SBT_OPTS \
+ $JAVA_OPTS \
+ -Djava.net.preferIPv4Stack=true \
+ -XX:+AggressiveOpts \
+ -XX:+UseParNewGC \
+ -XX:+UseConcMarkSweepGC \
+ -XX:+CMSParallelRemarkEnabled \
+ -XX:+CMSClassUnloadingEnabled \
+ -XX:MaxPermSize=1024m \
+ -XX:SurvivorRatio=128 \
+ -XX:MaxTenuringThreshold=0 \
+ -Xss8M \
+ -Xms512M \
+ -Xmx3G \
+ -server \
+ -jar $sbtjar "$@"
View
7 lib/template/src/main/scala/com/twitter/birdname/BirdNameServiceImpl.scala.erb
@@ -1,5 +1,6 @@
package com.twitter.birdname
+import com.twitter.conversions.time._
import com.twitter.logging.Logger
import com.twitter.util._
import java.util.concurrent.Executors
@@ -31,7 +32,7 @@ class BirdNameServiceImpl(config: BirdNameServiceConfig) extends BirdNameService
database.get(key) match {
case None =>
log.debug("get %s: miss", key)
- Future.exception(new BirdNameException("No such key"))
+ Future.exception(BirdNameException("No such key"))
case Some(value) =>
log.debug("get %s: hit", key)
Future(value)
@@ -43,4 +44,8 @@ class BirdNameServiceImpl(config: BirdNameServiceConfig) extends BirdNameService
database(key) = value
Future.Unit
}
+
+ def shutdown() = {
+ super.shutdown(0.seconds)
+ }
}
Something went wrong with that request. Please try again.