Version bumps BT-332 #6456

kshakir · 2021-07-25T23:52:43Z

No description provided.

- Wait longer for containers to download and startup - Use the latest PostgreSQL jdbc driver - Updated liquibase and schema comparison tests - Empty LOBs are always stored as null - Fixed test description for empty lobs - Test the various databases with centaur local - Don't start a db when CI checking publishing - Help liquibase by using valid location of S3FSP - Longer leeway for logging events for longer liquibase

- sbt 1.5.3 - sbt-assembly 1.0.0 - more assembly merging workarounds - disabled test log buffering - removed some intellij warnings and false-errors - only set dockerCustomSettings when required

- updated scala-collection-compat - removed obsolete merging exclusions - removed intellij warnings from Dependencies.scala

cjllanwarne · 2021-08-09T15:18:32Z

Just OOO, is this mostly complementary, orthogonal, or replacing-of, the scala steward update PRs?

kshakir · 2021-08-14T16:24:25Z

Just OOO, is this mostly complementary, orthogonal, or replacing-of, the scala steward update PRs?

I haven't gone through the various other PRs yet to see what they're fixing or not. Over weekends I've been experimenting with updating various subsystems I'm already familiar with and seeing if Travis likes the changes. If I had to guess, there's probably a bit of overlap with the version bumps here and the scala steward PRs.

Things done here, and may or may not have been addressed in the stewarded PRs:

Some non-semver versions have been updated/fixed. Does scala steward do date comparisons or only semver? (ex: nl.grons.metrics(3), apache commons, workbench-libs, etc.)
Some intermediate version fixes have been applied. The versions listed in develop will be out of date, while the absolute latest available version may not be compatible (ex: cats-effect, fs2, http4s, etc.)
Some SDKs had a few deprecated functions and required a little RTFMing 📖 (ex: sentry)

Btw, as it's not a blocker (yet) some weekend I or someone else will have to dive into statsd and deal with those libs plus whatever our bespoke statsd-proxy is doing... 🤔

mcovarr

👍 assuming Jenkins is okay with the AWS changes

mcovarr · 2021-08-30T19:39:27Z

.travis.yml

    - >-
      BUILD_TYPE=centaurPapiUpgradePapiV2alpha1
      BUILD_MYSQL=5.7
    - >-
      BUILD_TYPE=centaurPapiUpgradeNewWorkflowsPapiV2alpha1
      BUILD_MYSQL=5.7
+    - >-
+      BUILD_TYPE=centaurLocal
+      BUILD_MARIADB=10.3


2 horicromtal papis for a local seems like a deal

mcovarr · 2021-08-30T19:39:48Z

CromIAM/src/main/resources/sentry.properties

@@ -0,0 +1,2 @@
+# Quiet warnings about missing sentry DSNs by providing an empty string
+dsn=


mcovarr · 2021-08-30T20:09:07Z

project/Dependencies.scala

-  // It's possible that after updating all other dependencies in Cromwell we might purge the transitive dependencies on
-  // javax.activation and then be able to upgrade to jaxb-impl 2.3.3 or beyond, but some of those other dependencies
-  // such as googleCloudNioV have already been pinned for Scala Steward so this might not be a trivial undertaking.
-  private val jaxbV = "2.3.2" // scala-steward:off


mcovarr · 2021-08-30T20:54:01Z

services/src/test/scala/cromwell/services/database/LiquibaseComparisonSpec.scala

+  private val MariadbColumnMapping =
+    ColumnMapping(
+      typeMapping = Map(
+        HsqldbTypeBigInt -> ColumnType("BIGINT"),


these key names look kind of surprising given this is a Mariadb map...

The keys in the map are what HSQLDB calls the column. The values are what the other RDBMS calls the column. In this case MariaDB.

The way the matching works, at the start of the test an HSQLDB schema is created with Slick instead of Liquibase. Then one by one each RDBMS type is initialized, liquibased, and then the resulting schema is compared to the HSQLDB-Slick schema. Any mismatches in column type are an error. Except sometimes there are minor differences.

In this line HSQLDB has columns that are ColumnType("BIGINT", Option(64)) while MariaDB doesn't return sizes for "BIGINT" so its equivalent column type is a CoulmnType("BIGINT", None).

mcovarr · 2021-08-30T21:31:19Z

project/Dependencies.scala

  private val apacheHttpClientV = "4.5.13"
-  private val awsSdkV = "2.15.41" // scala-steward:off (CROM-6776)
+  private val awsSdkV = "2.17.29"


kicked off AWS build 104 to get Jenkins' opinion before merge

breilly2 · 2021-08-31T12:11:10Z

project/Dependencies.scala

+  // latest date via: https://mvnrepository.com/artifact/com.google.apis/google-api-services-cloudkms
+  private val googleCloudKmsV = "v1-rev20210812-1.32.1"
+  private val googleCloudMonitoringV = "3.0.2"
+  private val googleCloudNioV = "0.123.8"


Oof. This goes back to last August: https://github.com/broadinstitute/cromwell/pull/5708/files#diff-ad2642dc77b3679e4ad2c7d3b2f59a2dcf48c1bf5b93a558a4fa44828315a34cR35. Maybe this one required code changes?

breilly2 · 2021-08-31T12:24:55Z

project/Dependencies.scala

+  // latest date via: https://mvnrepository.com/artifact/com.google.apis/google-api-services-lifesciences
+  private val googleLifeSciencesServicesV2BetaApiV = "v2beta-rev20210813-1.32.1"


breilly2 · 2021-08-31T12:36:41Z

project/Dependencies.scala

-  // It's possible that after updating all other dependencies in Cromwell we might purge the transitive dependencies on
-  // javax.activation and then be able to upgrade to jaxb-impl 2.3.3 or beyond, but some of those other dependencies
-  // such as googleCloudNioV have already been pinned for Scala Steward so this might not be a trivial undertaking.
-  private val jaxbV = "2.3.2" // scala-steward:off


If this all works, I love getting rid of pinned versions like this.

breilly2 · 2021-08-31T12:47:06Z

project/Dependencies.scala

-  private val owlApiV = "5.1.16" // scala-steward:off (CROM-6677)
+  private val mockserverNettyV = "5.11.2"
+  private val mouseV = "1.0.4"
+  private val mysqlV = "8.0.26"


Some stuff about character sets: https://dev.mysql.com/doc/relnotes/connector-j/8.0/en/news-8-0-26.html#connector-j-8-0-26-feature. Anything to note here?

Character encodings, like date-times, are definitely something to keep an eye out for.

In general anyone not using UTC time and 7-bit ASCII may run into issues 😦

That said, we generally hope that any egregious encoding bugs are caught in centaur. For the curious, bugs in the past have popped up when validating and storing values of length "255" in our liquibased tables. What unit is that: bytes, characters, encoded characters in bytes?? If my memory is correct, the issues manifested as strings suddenly being too long to store in some of the tables.

breilly2 · 2021-08-31T12:56:58Z

project/Dependencies.scala

-  private val refinedV = "0.9.22"
+  private val postgresV = "42.2.23"
+  private val pprintV = "0.6.6"
+  private val rdf4jV = "3.7.1"


Ugh. Their link to the "release and upgrade notes" (https://rdf4j.org/release-notes#3-0-0) for 3.0.0 doesn't go to a useful place anymore so I don't know what to look for. I also don't know what RDF is or how we might use it.

RDF: https://en.wikipedia.org/wiki/Resource_Description_Framework

It's one of the specs used by the CWL spec (example)

This repo only uses rdf4j transitively. However, there was a vulnerability detected in an earlier version, and the OWL API dependency (yet another W3C spec) we use was slow to update their dependencies.

For the curious about these libs in general there are more external links regarding dependencies here

breilly2 · 2021-08-31T17:15:20Z

project/Dependencies.scala

  private val scoptV = "4.0.1"
-  private val sentryLogbackV = "1.7.30" // scala-steward:off (CROM-6640)
-  private val shapelessV = "2.3.3"
+  private val sentryLogbackV = "5.1.2"


This feels like a huge jump and I wouldn't even know where to start, so... just saying that I'm leaving this one alone and assuming that any problems would be caught by compilation, tests, or general operations.

breilly2 · 2021-08-31T17:50:25Z

project/Dependencies.scala

-  private val snakeyamlV = "1.28"
-  private val specs2MockV = "4.10.6"
+  private val snakeyamlV = "1.29"
+  private val specs2MockV = "4.12.3"


I care about this, but I cannot find any reliable documentation links about specs2-mock or any version history. I hope they didn't change anything that will cause any of our tests to pass when they should fail.

I care too. The reason it's hard to find docs on specs2-mock is that it seems to be End Of Life. See the cromwell sibling PR that kills this dependency off 😉🔪💀

As for some sort of docs:

4.12.0 mentions specs2's specs2-mock extension: https://etorreborre.github.io/specs2/guide/SPECS2-4.12.0/org.specs2.guide.Installation.html#other-dependencies

5.0 (in RC) no longer mentions the extensions and instead refers to scalamock: https://etorreborre.github.io/specs2/guide/5.0.0-RC-01/org.specs2.guide.Installation.html#other-dependencies

breilly2 · 2021-08-31T17:53:06Z

project/Dependencies.scala

-  private val workbenchModelV = "0.14-27810079-SNAP"
-  private val workbenchUtilV = "0.6-27810079-SNAP"


-SNAP ⁉️ 😬

Desperate times... (and not waiting for releases) 🤷

breilly2 · 2021-08-31T18:03:36Z

filesystems/oss/src/main/scala/cromwell/filesystems/oss/batch/OssBatchIoCommand.scala

@@ -57,7 +57,10 @@ case class OssBatchDeleteCommand(
                                 override val file: OssPath,
                                 override val swallowIOExceptions: Boolean
                               ) extends IoDeleteCommand(file, swallowIOExceptions) with OssBatchIoCommand[Unit, Void] {
-  def operation: Unit = file.ossClient.deleteObject(file.bucket, file.key)
+  def operation: Unit = {
+    file.ossClient.deleteObject(file.bucket, file.key)


Does this not return void anymore? Should we be paying attention to what it does return?

Should we be paying attention to what it does return?

No. It returns a VoidResult: https://github.com/aliyun/aliyun-oss-java-sdk/blob/3.13.1/src/main/java/com/aliyun/oss/OSSClient.java#L665

breilly2 · 2021-08-31T18:04:10Z

filesystems/oss/src/test/scala/cromwell/filesystems/oss/nio/OssNioUtilSpec.scala

@@ -98,6 +98,7 @@ trait OssNioUtilSpec extends AnyFlatSpecLike with CromwellTimeoutSpec with Mocki
    OssStorageRetry.from(
      () => ossClient.deleteObject(path.bucket, path.key)
    )
+    ()


Same question as in OssBatchIoCommand.scala above.

breilly2 · 2021-08-31T18:08:44Z

...src/test/scala/cromwell/engine/workflow/lifecycle/execution/WorkflowExecutionActorSpec.scala

@@ -27,7 +27,7 @@ import scala.concurrent.{Await, Promise}
 class WorkflowExecutionActorSpec extends CromwellTestKitSpec with AnyFlatSpecLike with Matchers with BeforeAndAfter with WorkflowDescriptorBuilderForSpecs {

  override implicit val actorSystem = system
-  implicit val DefaultDuration = 20.seconds.dilated
+  implicit val DefaultDuration = 60.seconds.dilated


Why this change?

This spec, like others, effectively runs its own entire Cromwell instance. When run on Travis it sometimes takes longer than 20 seconds for the lazy-loaded in-memory DB to start up, especially when other tests don't clean 100% clean up leading to GC pauses etc.

See the comment "Yes, this might be slow..." further down below in this file for more slightly more context.

breilly2 · 2021-08-31T18:34:57Z

services/src/test/scala/cromwell/services/database/LobSpec.scala

@@ -35,7 +35,7 @@ class LobSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers with Sc
      containerOpt.foreach { _.start }
    }

-    it should "fail to store and retrieve empty blobs" taggedAs DbmsTest in {
+    it should "store empty blobs" taggedAs DbmsTest in {


Was this just always incorrectly named?

Not "always". At some point the body of the test was updated and the name of the test wasn't.

breilly2

Looks good, as far as I can tell. I've done what I can to validate version bumps, and I'm trusting that the code changes are largely fallout from that. I also don't know the newer sbt stuff, so I'm trusting that that's all correct. Definitely glad there are other eyes on reviewing this.

kshakir added 19 commits May 2, 2021 19:43

Scala 2.12.13

c20cb5c

Merge branch 'develop' into ks_2.12.13

64a7a6c

PR Fixup: 2.12.14 was released

89fc0f4

PR Fixup: sbt updates:

884b60c

- sbt 1.5.3 - sbt-assembly 1.0.0 - more assembly merging workarounds - disabled test log buffering - removed some intellij warnings and false-errors - only set dockerCustomSettings when required

Merge branch 'develop' into ks_2.12.13

cf6c8a1

PR Fixup: sbt 1.5.4 released

c8612fc

Merge branch 'develop' into ks_db_patches

87bad49

PR Fixup: Remove cosmetic patch added in another PR

ad20b9a

Merge branch 'develop' into ks_2.12.13

03ba4f9

PR fixup:

cc44ec5

- updated scala-collection-compat - removed obsolete merging exclusions - removed intellij warnings from Dependencies.scala

Merge branch 'develop' into ks_2.12.13

0d9444f

PR Fixup: sbt 1.5.5 released

c1d5b46

Merge branch 'ks_2.12.13' into ks_version_bumps

ff72660

Merge branch 'ks_db_patches' into ks_version_bumps

49ac5a6

Version bumps

56d20b1

PR Fixup: Temporaliy downgrade liquibase

a016112

PR Fixup: Bump sdkman jdk

87ac4ed

PR Fixup: More time for impatient tests

2856d87

kshakir force-pushed the ks_version_bumps branch from ecdccd7 to 2856d87 Compare July 26, 2021 03:53

kshakir added 3 commits July 26, 2021 08:29

PR Fixup: Patch timing issue by adding another global var

aabcd9f

Merge branch 'develop' into ks_version_bumps

f488579

PR Fixup: More bumps

f61f182

kshakir added 2 commits August 14, 2021 04:15

Merge branch 'develop' into ks_version_bumps

66240c8

PR Fixup: More bumps

71ab92d

kshakir added 3 commits August 16, 2021 04:14

PR Fixup: Upgrade from Metrics 3.x to 4.x

9bcfba6

PR Fixup: More bumps plus patience in KeyValueServiceActorSpec

9dc427c

PR Fixup: More bumps plus fix AWS assembly issue

70c840d

kshakir marked this pull request as ready for review August 30, 2021 20:32

kshakir requested review from mcovarr and breilly2 August 30, 2021 20:32

mcovarr approved these changes Aug 30, 2021

View reviewed changes

breilly2 reviewed Aug 31, 2021

View reviewed changes

breilly2 approved these changes Aug 31, 2021

View reviewed changes

kshakir merged commit fdedcbb into develop Aug 31, 2021

kshakir deleted the ks_version_bumps branch August 31, 2021 21:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Version bumps BT-332 #6456

Version bumps BT-332 #6456

kshakir commented Jul 25, 2021

cjllanwarne commented Aug 9, 2021

kshakir commented Aug 14, 2021

mcovarr left a comment

mcovarr Aug 30, 2021

mcovarr Aug 30, 2021

mcovarr Aug 30, 2021

mcovarr Aug 30, 2021

kshakir Aug 30, 2021

mcovarr Aug 30, 2021

breilly2 Aug 31, 2021

breilly2 Aug 31, 2021

breilly2 Aug 31, 2021

breilly2 Aug 31, 2021

kshakir Aug 31, 2021

breilly2 Aug 31, 2021

kshakir Aug 31, 2021

breilly2 Aug 31, 2021

breilly2 Aug 31, 2021

kshakir Aug 31, 2021

breilly2 Aug 31, 2021

kshakir Aug 31, 2021

breilly2 Aug 31, 2021

kshakir Aug 31, 2021

breilly2 Aug 31, 2021

breilly2 Aug 31, 2021

kshakir Aug 31, 2021

breilly2 Aug 31, 2021

kshakir Aug 31, 2021

breilly2 left a comment

		@@ -0,0 +1,2 @@
		# Quiet warnings about missing sentry DSNs by providing an empty string
		dsn=

		// latest date via: https://mvnrepository.com/artifact/com.google.apis/google-api-services-lifesciences
		private val googleLifeSciencesServicesV2BetaApiV = "v2beta-rev20210813-1.32.1"

		private val workbenchModelV = "0.14-27810079-SNAP"
		private val workbenchUtilV = "0.6-27810079-SNAP"

Version bumps BT-332 #6456

Version bumps BT-332 #6456

Conversation

kshakir commented Jul 25, 2021

cjllanwarne commented Aug 9, 2021

kshakir commented Aug 14, 2021

mcovarr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

breilly2 left a comment

Choose a reason for hiding this comment