Spot-166 module code cleanup pass #60
Conversation
… be sparkSession.
Unit testing added to improve coverage, current lines covered 84%.
| * @param ldaAlpha LDA alpha parameter | ||
| * @param ldaBeta LDA beta parameter | ||
| * @param ldaOptimizerOption LDA optimizer, em or online | ||
| * @param maxIterations number of maximum iteration LDA will run |
There was a problem hiding this comment.
Should this say, "maximum number of iterations for the optimizer" or something similar? The current line is a little unclear to me.
There was a problem hiding this comment.
I like your suggestion, thanks.
There was a problem hiding this comment.
ldaAlpha - shouldn't this be explained as document concentration
ldaBeta - should this be explained as topic concentration?
| @@ -66,6 +75,13 @@ object WideUDFs { | |||
| :: Nil).getOrElse(Nil) | |||
| UserDefinedFunction(f, ScalaReflection.schemaFor(typeTag[RT]).dataType, Option(inputTypes)) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Thanks for adding these comments.
| @@ -32,7 +32,7 @@ object DNSFeedback { | |||
| /** | |||
| * Load the feedback file for DNS data. | |||
| * | |||
| * @param spark Spark Session | |||
| * @param sparkSession Spark Session | |||
| @@ -150,9 +150,12 @@ class ProxySuspiciousConnectsAnalysisTest extends TestingSparkContextFlatSpec wi | |||
| val data = sparkSession.createDataFrame(Seq(anomalousRecord, typicalRecord, typicalRecord, typicalRecord, typicalRecord, | |||
| typicalRecord, typicalRecord, typicalRecord, typicalRecord, typicalRecord)) | |||
|
|
|||
| val model = ProxySuspiciousConnectsModel.trainModel(sparkSession, logger, onlineTestConfigProxy, data) | |||
| // val model = ProxySuspiciousConnectsModel.trainModel(sparkSession, logger, onlineTestConfigProxy, data) | |||
There was a problem hiding this comment.
Why are these lines commented?
There was a problem hiding this comment.
Yeah, my bad. Thank you and @brandon-edwards for noticing this.
| @@ -19,6 +19,9 @@ package org.apache.spot.lda | |||
|
|
|||
| import org.apache.spark.sql.types.{LongType, StringType, StructField} | |||
|
|
|||
| /** | |||
| * Schemas and column names used in SpotLDAWrapper and *SuspiciousConnectsModel. | |||
There was a problem hiding this comment.
is there any meaning to the * next to SuspiciousConnectsModel ?
There was a problem hiding this comment.
- --> Any pipeline, Flow, DNS or Proxy.
There was a problem hiding this comment.
Oh. Since there's just three right now, you might as well list them. As it is, it looks like a typo.
| * | ||
| * @param domain domain name | ||
| * @return | ||
| */ | ||
| def domainBelongsToSafeList(domain: String) = domain == "intel" // TBD parameterize this! |
There was a problem hiding this comment.
this returns a Booelan - the return type should be noted in the signature and the @return parameter should informally describe the result
|
Done. |
Implements changes requested on SPOT-166
Main changes