update to jdk 11 [AS-594] #1372
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 11 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| FROM us.gcr.io/broad-dsp-gcr-public/base/jre:11-debian | ||
|
|
||
| # To run, must build the jar using ./docker/build.sh | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| FROM broadinstitute/scala-baseimage:jdk11-2.12.12-1.4.7 | ||
|
|
||
| COPY src /app/src | ||
| COPY minnie-kenny.sh /app | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| sbt.version = 1.4.7 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,7 +87,7 @@ object Dependencies { | |
| val scalaUri: ModuleID = "io.lemonlabs" %% "scala-uri" % "3.0.0" | ||
| val scalatest: ModuleID = "org.scalatest" %% "scalatest" % "3.2.2" % "test" | ||
| val mockito: ModuleID = "org.scalatestplus" %% "mockito-3-4" % "3.2.2.0" % Test | ||
| val mockserverNetty: ModuleID = "org.mock-server" % "mockserver-netty" % "5.11.2" % "test" | ||
zarsky-broad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| val ficus: ModuleID = "com.iheart" %% "ficus" % "1.4.0" | ||
| val scalaCache: ModuleID = "com.github.cb372" %% "scalacache-caffeine" % "0.24.2" | ||
| val apacheCommonsIO: ModuleID = "commons-io" % "commons-io" % "2.6" | ||
|
|
@@ -108,8 +108,10 @@ object Dependencies { | |
|
|
||
| val accessContextManager = "com.google.apis" % "google-api-services-accesscontextmanager" % "v1beta-rev55-1.25.0" | ||
|
|
||
| def swaggerClientExcludes(m: ModuleID): ModuleID = m.exclude("jakarta.activation", "jakarta.activation-api") | ||
|
|
||
| val workspaceManager = swaggerClientExcludes("bio.terra" % "workspace-manager-client" % "0.13.0-SNAPSHOT") | ||
| val dataRepo = swaggerClientExcludes("bio.terra" % "datarepo-client" % "1.0.44-SNAPSHOT") | ||
|
There was a problem hiding this comment. @kshakir @davidangb @asingh7115 Preferable? There was a problem hiding this comment. much better, glad this is working via excludes! nit, optional: rename lastly: do these no longer need to exclude guava-jdk5? There was a problem hiding this comment. They don't actually include guava-jdk5, that's not included in anything vaguely recent. |
||
|
|
||
| val opencensusScalaCode: ModuleID = "com.github.sebruck" %% "opencensus-scala-core" % "0.7.0-M2" | ||
| val opencensusAkkaHttp: ModuleID = "com.github.sebruck" %% "opencensus-scala-akka-http" % "0.7.0-M2" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import MinnieKenny.testSettings | ||
| import sbt.Keys._ | ||
| import sbt._ | ||
|
|
||
|
|
@@ -22,6 +23,9 @@ object Testing { | |
|
|
||
| val commonTestSettings: Seq[Setting[_]] = List( | ||
|
|
||
| testOptions in Test += Tests.Setup(() => | ||
| sys.props += "mockserver.logLevel" -> "WARN" | ||
|
There was a problem hiding this comment. The default log verbosity for mockserver logs every mock defined, for every test that defines it. Since this goes to the console for jenkins jobs, it makes the logs huge, to the point that the browser has trouble searching through them. |
||
| ), | ||
| // SLF4J initializes itself upon the first logging call. Because sbt | ||
| // runs tests in parallel it is likely that a second thread will | ||
| // invoke a second logging call before SLF4J has completed | ||
|
|
@@ -55,7 +59,7 @@ object Testing { | |
| (testOnly in Test) := ((testOnly in Test) dependsOn validMySqlHost).evaluated, | ||
|
|
||
| parallelExecution in Test := false | ||
| ) ++ (if (sys.props.getOrElse("secrets.skip", false) != "true") MinnieKenny.testSettings else List()) | ||
|
There was a problem hiding this comment. I chose this option name somewhat arbitrarily, if it doesn't make sense it can be changed. Alternatively, we could remove Minnie-Kenny being run by sbt; Orch doesn't have it, and it's run manually in the github CI. There was a problem hiding this comment. To confirm: Orch doesn't check for the secrets leaks commited in There was a problem hiding this comment. Orch doesn't have any minnie-kenny anything of any kind; I'm not aware of any discussion around it, @davidangb do you know of anything? There was a problem hiding this comment. I don't see any tickets in the backlog. It is used by most other services (like rawls and sam) so maybe we should add it There was a problem hiding this comment. for my clarification: this discussion of orch is tangential to this PR, right? yeah, if orch does not have secrets-detection, it absolutely needs it. I'd prefer to check with appsec first on the progress of their centralized/cross-repo secrets detection before committing to any work, if appsec is already handling it let's just let them. There was a problem hiding this comment. I think it's tangential. The question came from the switch to skip testing for git-secrets via the line above that says There was a problem hiding this comment. @zarsky-broad please confirm, but my understanding is this toggle is specific to Rawls, and is called from the change in this PR in docker/install.sh. There was a problem hiding this comment. Yeah. The old build image for Rawls didn't include git at all, so the secrets check was skipped silently. The new image has git, so the secrets check was failing here because it didn't have git-secrets. I added an optional flag to skip it that's only used in the The alternative is to not have sbt run the check at all, since it's explicitly run in the Github CI workflow. There was a problem hiding this comment. Ah, I see the problem:
Orch has no minnie-kenny at all. Separately, the rawls github workflow runs minnie-kenny. There was a problem hiding this comment. I think there's a scenario here where you're comparing a Boolean to a String. My quick test in the repl shows that this can cause problems. Recommend coercing everything the same type. |
||
|
|
||
| implicit class ProjectTestSettings(val project: Project) extends AnyVal { | ||
| def withTestSettings: Project = project | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| sbt.version=1.4.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming container-based compilation is absolutely required, someday I hope this custom image gets pushed to a public GCR or anywhere else besides docker.io. We have so many tests in the terra multi-repo that are failing due to pull rate limits from docker.io these days. 😢
Or for easier patching of individual components, just use a more flexible base container like focal and then download java and the 0.016GB sbt package. While Java gets patches much slower, Scala and SBT get patches once a month. For example: sbt 1.4.8 is already out.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my take is that even though sbt will automatically download whatever it needs, the entire point of adding sbt to a baseimage is to preload as many dependencies as possible at docker-build time, and avoid re-downloading those dependencies on every build of every service that uses this image. So, counting on sbt to download what it needs defeats the purpose - if we're relying on that, I could see omitting it entirely and downloading it at runtime, avoiding the maintenance of the baseimage.