config tweaks (#12)

* (auto) update dependency lock file, version=2.2.0-dev.0.uncommitted+config.load.a930b1f * (auto) update dependency lock file, version=2.2.0-dev.1.uncommitted+config.load.3f7f8e7 * remove unused properties, dont use deprecated config methods * properties tweaks and template * config tweaks * additional doc * use xenial dist * gradle stacktrace * use default dist * additional tests * additional tests * doc clarification * more tests * remove unused constructor
Workday · Nov 19, 2018 · faab3f7 · faab3f7
1 parent a930b1f
commit faab3f7
Show file tree

Hide file tree

Showing 16 changed files with 331 additions and 112 deletions.
diff --git a/.gitignore b/.gitignore
@@ -16,3 +16,4 @@ classes
 out
 public
 gh-pages-src/static/scaladoc/warp-core
+config
diff --git a/.travis.yml b/.travis.yml
@@ -23,7 +23,7 @@ cache:
 
 script:
   - docker-compose up -d
-  - ./gradlew -PallScalaVersions reportScoverage checkScoverage
+  - ./gradlew -PallScalaVersions copyConfigTemplate reportScoverage checkScoverage --stacktrace
 
 after_success:
   - ./gradlew coveralls
diff --git a/build.gradle b/build.gradle
@@ -85,6 +85,7 @@ dependencies {
     scoverage "org.scoverage:scalac-scoverage-runtime_%%:${versions.scalacScoverage}"
 
     runtimeOnly "com.h2database:h2:${versions.h2}"
+    runtimeOnly "commons-beanutils:commons-beanutils:${versions.beanUtils}"
     runtimeOnly "mysql:mysql-connector-java:${versions.mysqlConnector}"
     runtimeOnly "org.tinylog:slf4j-binding:${versions.tinylogSlf4jBinding}"
 }
@@ -139,6 +140,19 @@ scaladoc {
     destinationDir file("$rootDir/gh-pages-src/static/scaladoc/${project.name}")
 }
 
+// copy config template into a usable place
+// note this task does not have any dependencies, and nothing depends on this task.
+// the expectation is this task will always be run in a ci environment,
+// and can be run once locally on a fresh checkout to create a starting config point
+task copyConfigTemplate(type: Copy) {
+    from "config-templates/warp.properties.template"
+    rename { String filename ->
+        filename - '.template'
+    }
+
+    into "config/"
+}
+
 task unitTest(type: Test) {
     useJUnit {
         excludeCategories 'com.workday.warp.common.category.IntegrationTest'

diff --git a/warp.properties → config-templates/warp.properties.template b/warp.properties → config-templates/warp.properties.template
@@ -1,8 +1,13 @@
+# template config file used in ci builds
+# this file is copied into config directory
+# for local development, this template can be used as a starting point for more customized settings
 wd.warp.jdbc.url=jdbc:mysql://localhost:3307/warp_core?createDatabaseIfNotExist=true&zeroDateTimeBehavior=convertToNull&verifyServerCertificate=false&useSSL=true
 # make sure to use something more secure in production
 wd.warp.jdbc.password=1234
 wd.warp.jdbc.user=root
 wd.warp.jdbc.driver=com.mysql.jdbc.Driver
+# TODO seems slower in travis than locally. investigate.
+wd.warp.jdbc.timeout=300
 wd.warp.migrate.schema=true
 
 wd.warp.log.level=info

diff --git a/dependencies_2.11.lock b/dependencies_2.11.lock
@@ -1352,13 +1352,24 @@
                 "com.typesafe.slick:slick-hikaricp_2.11"
             ]
         },
+        "commons-beanutils:commons-beanutils": {
+            "locked": "1.9.3",
+            "requested": "1.9.3"
+        },
+        "commons-collections:commons-collections": {
+            "locked": "3.2.2",
+            "transitive": [
+                "commons-beanutils:commons-beanutils"
+            ]
+        },
         "commons-io:commons-io": {
             "locked": "2.6",
             "requested": "2.6"
         },
         "commons-logging:commons-logging": {
             "locked": "1.2",
             "transitive": [
+                "commons-beanutils:commons-beanutils",
                 "org.apache.commons:commons-configuration2"
             ]
         },
@@ -2440,13 +2451,24 @@
                 "com.typesafe.slick:slick-hikaricp_2.11"
             ]
         },
+        "commons-beanutils:commons-beanutils": {
+            "locked": "1.9.3",
+            "requested": "1.9.3"
+        },
+        "commons-collections:commons-collections": {
+            "locked": "3.2.2",
+            "transitive": [
+                "commons-beanutils:commons-beanutils"
+            ]
+        },
         "commons-io:commons-io": {
             "locked": "2.6",
             "requested": "2.6"
         },
         "commons-logging:commons-logging": {
             "locked": "1.2",
             "transitive": [
+                "commons-beanutils:commons-beanutils",
                 "org.apache.commons:commons-configuration2"
             ]
         },
@@ -4630,13 +4652,24 @@
                 "com.typesafe.slick:slick-hikaricp_2.11"
             ]
         },
+        "commons-beanutils:commons-beanutils": {
+            "locked": "1.9.3",
+            "requested": "1.9.3"
+        },
+        "commons-collections:commons-collections": {
+            "locked": "3.2.2",
+            "transitive": [
+                "commons-beanutils:commons-beanutils"
+            ]
+        },
         "commons-io:commons-io": {
             "locked": "2.6",
             "requested": "2.6"
         },
         "commons-logging:commons-logging": {
             "locked": "1.2",
             "transitive": [
+                "commons-beanutils:commons-beanutils",
                 "org.apache.commons:commons-configuration2"
             ]
         },

diff --git a/dependencies_2.12.lock b/dependencies_2.12.lock
@@ -1343,13 +1343,24 @@
                 "com.typesafe.slick:slick-hikaricp_2.12"
             ]
         },
+        "commons-beanutils:commons-beanutils": {
+            "locked": "1.9.3",
+            "requested": "1.9.3"
+        },
+        "commons-collections:commons-collections": {
+            "locked": "3.2.2",
+            "transitive": [
+                "commons-beanutils:commons-beanutils"
+            ]
+        },
         "commons-io:commons-io": {
             "locked": "2.6",
             "requested": "2.6"
         },
         "commons-logging:commons-logging": {
             "locked": "1.2",
             "transitive": [
+                "commons-beanutils:commons-beanutils",
                 "org.apache.commons:commons-configuration2"
             ]
         },
@@ -2421,13 +2432,24 @@
                 "com.typesafe.slick:slick-hikaricp_2.12"
             ]
         },
+        "commons-beanutils:commons-beanutils": {
+            "locked": "1.9.3",
+            "requested": "1.9.3"
+        },
+        "commons-collections:commons-collections": {
+            "locked": "3.2.2",
+            "transitive": [
+                "commons-beanutils:commons-beanutils"
+            ]
+        },
         "commons-io:commons-io": {
             "locked": "2.6",
             "requested": "2.6"
         },
         "commons-logging:commons-logging": {
             "locked": "1.2",
             "transitive": [
+                "commons-beanutils:commons-beanutils",
                 "org.apache.commons:commons-configuration2"
             ]
         },
@@ -4591,13 +4613,24 @@
                 "com.typesafe.slick:slick-hikaricp_2.12"
             ]
         },
+        "commons-beanutils:commons-beanutils": {
+            "locked": "1.9.3",
+            "requested": "1.9.3"
+        },
+        "commons-collections:commons-collections": {
+            "locked": "3.2.2",
+            "transitive": [
+                "commons-beanutils:commons-beanutils"
+            ]
+        },
         "commons-io:commons-io": {
             "locked": "2.6",
             "requested": "2.6"
         },
         "commons-logging:commons-logging": {
             "locked": "1.2",
             "transitive": [
+                "commons-beanutils:commons-beanutils",
                 "org.apache.commons:commons-configuration2"
             ]
         },

diff --git a/scalastyle.xml b/scalastyle.xml
@@ -149,7 +149,7 @@
  </check>
  <check customId="two.spaces" enabled="true" class="org.scalastyle.file.RegexChecker" level="error">
   <parameters>
-   <parameter name="regex">[^\s]  [^\s]</parameter>
+   <parameter name="regex">[^\s/]  [^\s]</parameter>
    <parameter name="line">false</parameter>
   </parameters>
   <customMessage>tokens should not be separated by two spaces</customMessage>

diff --git a/src/main/scala/com/workday/warp/common/HasCoreWarpProperties.scala b/src/main/scala/com/workday/warp/common/HasCoreWarpProperties.scala
@@ -55,6 +55,16 @@ trait HasCoreWarpProperties extends WarpPropertyLike {
     */
   val WARP_DATABASE_DRIVER: PropertyEntry = PropertyEntry("wd.warp.jdbc.driver", isRequired = true, "org.h2.Driver")
 
+  /**
+    * Timeout for DB queries in seconds.
+    *
+    * We'll await slick futures for this duration.
+    *
+    * Required: Yes
+    * Default Value: 90
+    */
+  val WARP_DATABASE_TIMEOUT: PropertyEntry = PropertyEntry("wd.warp.jdbc.timeout", isRequired = true, "90")
+
   /**
     * Whether or not we should apply flyway schema migrations.
     *
@@ -433,32 +443,6 @@ trait HasCoreWarpProperties extends WarpPropertyLike {
     "wd.warp.zscore.percentile.arbiter.enabled", isRequired = false, "false"
   )
 
-  /**
-    * Whether anomaly detection provided by lambda function is enabled.
-    *
-    * Required: Yes
-    * Default Value: false
-    */
-  val WARP_LAMBDA_ARBITER_ENABLED: PropertyEntry = PropertyEntry("wd.warp.lambda.arbiter.enabled", isRequired = true, "false")
-
-  /**
-    * Fully qualified URL for anomaly detection lambda function.
-    *
-    * Required: Yes
-    * Default Value: https://nsd0jwpj65.execute-api.us-west-1.amazonaws.com/prod/anomaly_detection
-    */
-  val WARP_LAMBDA_ARBITER_URL: PropertyEntry = PropertyEntry(
-    "wd.warp.lambda.arbiter.url", isRequired = true, "https://nsd0jwpj65.execute-api.us-west-1.amazonaws.com/prod/anomaly_detection"
-  )
-
-  /**
-    * API key for the API that invokes the anomaly detection lambda function
-    *
-    * Required: Yes
-    * Default Value: None
-    */
-  val WARP_LAMBDA_ARBITER_API_KEY: PropertyEntry = PropertyEntry("wd.warp.lambda.arbiter.api.key", isRequired = true)
-
   /**
     * Threshold used by the percentile (gaussian distribution z-score) arbiter.
     *

diff --git a/src/main/scala/com/workday/warp/common/WarpPropertyManager.scala b/src/main/scala/com/workday/warp/common/WarpPropertyManager.scala
@@ -1,18 +1,19 @@
 package com.workday.warp.common
 
-import java.io.{File, FileReader}
+import java.io.{File, FileNotFoundException, FileReader}
 import java.util.Properties
 
 import com.workday.warp.common.exception.WarpConfigurationException
 import com.workday.warp.inject.WarpGuicer
 import com.workday.warp.logger.WarpLogUtils
 import org.apache.commons.configuration2.PropertiesConfiguration
+import org.apache.commons.configuration2.builder.fluent.Configurations
 import org.apache.commons.configuration2.ex.ConfigurationException
 import org.pmw.tinylog.Logger
 
 import scala.collection.JavaConverters._
 import scala.collection.mutable
-import scala.util.Try
+import scala.util.{Failure, Success, Try}
 
 /**
   * Manages runtime-determination of warp configuration properties.
@@ -35,9 +36,6 @@ object WarpPropertyManager {
   private val WARP_PROPERTIES: String = "warp.properties"
   private val WARP_CONFIG_DIRECTORY_PROPERTY: String = "wd.warp.config.directory"
 
-
-  // load properties from the property file into this configuration
-  val configuration: PropertiesConfiguration = new PropertiesConfiguration
   // immutable copy of jvm system properties
   val systemProps: Map[String, String] = System.getProperties.asScala.toMap
   // determine the directory to search for warp configuration files
@@ -46,9 +44,20 @@ object WarpPropertyManager {
   val propertyFile: String = computePropertyFile
 
   // load the configuration file
-  Try(configuration.read(new FileReader(propertyFile))) recover {
-    case exception: ConfigurationException => Logger.error(exception, s"Error loading WARP Configuration file: $propertyFile \n\n")
-  }
+  // note that if the file does not exist, we log a warning and continue execution with an empty property set.
+  // (all default property values will be used)
+  // if there is an unrecoverable exception from configuration library, we'll throw that exception
+  val configuration: PropertiesConfiguration = Try(new Configurations().properties(propertyFile)).recoverWith {
+    case _: ConfigurationException if !new File(propertyFile).exists() =>
+      Logger.warn(s"$propertyFile does not exist!" +
+        "\n    Be aware that if the property values you require have not been passed in as Java System properties" +
+        "\n    this program is very likely to fail when an unset required property is accessed.\n")
+      // fall back on empty config
+      Success(new PropertiesConfiguration)
+    case exception: Exception =>
+      Logger.error(exception, s"Error loading WARP Configuration file: $propertyFile \n\n")
+      Failure(exception)
+  }.get
 
   // use di to see which property set we are working with
   val propertyEntries: Seq[PropertyEntry] = WarpGuicer.getProperty.values
@@ -224,16 +233,8 @@ object WarpPropertyManager {
     val versionedPropertyFile: File = new File(s"$default-$version")
 
     // check for a properties file declared with an extension that matches the current version
-    val propertyFile: String = if (versionedPropertyFile.exists) versionedPropertyFile.getAbsolutePath else default
-
-    // if the warp properties file does not exist, log a warning message
-    if (!new File(propertyFile).exists) {
-      Logger.warn(s"$propertyFile does not exist!" +
-        "\n    Be aware that if the property values you require have not been passed in as Java System properties" +
-        "\n    this program is very likely to fail when an unset required property is accessed.\n")
-    }
-
-    propertyFile
+    if (versionedPropertyFile.exists) versionedPropertyFile.getAbsolutePath
+    else default
   }
 
 
@@ -243,12 +244,12 @@ object WarpPropertyManager {
    *
    * 1. If wd.warp.config.directory Java system property is supplied in the TeamCity build configuration, look for
    * warp.properties file in that location.
-   * 2. If wd.warp.config.directory is not defined, check if warp.properties exists in the current working directory
-   * (i.e. /data/teamcity/buildAgent1140/work/{unique job identifier}/{module}/warp.properties).
+   * 2. If wd.warp.config.directory is not defined, check if ./config/warp.properties exists
+   * (i.e. /data/teamcity/buildAgent1140/work/{unique job identifier}/{module}/config/warp.properties).
    * 3. Finally, look for a warp.properties file in the users home .warp directory (i.e. /home/teamcity/.warp)
    */
   def computeConfigDirectory: String = {
-    val propertyFile: File = new File(this.WARP_PROPERTIES)
+    val propertyFile: File = new File(s"config/${this.WARP_PROPERTIES}")
 
     // check the jvm system property
     if (this.systemProps.contains(this.WARP_CONFIG_DIRECTORY_PROPERTY)) {

diff --git a/src/main/scala/com/workday/warp/common/heaphistogram/HeapHistogramEntry.scala b/src/main/scala/com/workday/warp/common/heaphistogram/HeapHistogramEntry.scala
@@ -1,27 +1,12 @@
 package com.workday.warp.common.heaphistogram
 
-import javax.management.openmbean.CompositeData
-
 /**
  * Small container class for class usage in a heap histogram.
  *
  * Created by tomas.mccandless on 9/21/15.
  */
-class HeapHistogramEntry(val className: String, val numInstances: Long, val numBytes: Long) {
+class HeapHistogramEntry(val className: String, val numInstances: Long, val numBytes: Long)
 
-  /**
-   * Auxiliary constructor accepting a CompositeData object
-   *
-   * @param histogramEntryCD CompositeData to construct a HeapHistogramEntry from
-   */
-  def this(histogramEntryCD: CompositeData) = {
-    this(
-      histogramEntryCD.get("className").toString,
-      histogramEntryCD.get("numInstances").toString.toLong,
-      histogramEntryCD.get("numBytes").toString.toLong
-    )
-  }
-}
 
 // compare entries based on their number of instances
 object InstancesOrdering extends Ordering[HeapHistogramEntry] {