-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(services): add kind and params #13
Conversation
app/Global.scala
Outdated
import play.api.Configuration | ||
import scala.concurrent.duration._ | ||
import scala.concurrent.ExecutionContext | ||
import play.api.{Configuration, _} |
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.
Maybe just import play.api._
then? (or maybe import only what you actually need)
app/models/OrderService.scala
Outdated
checks.map(checkToOrder(service)) | ||
check.kind match { | ||
case HTTP => { | ||
val params = check.params.getOrElse[Params](HTTPCheckParams(true, "/")) match { |
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.
The pattern matching is unnecessary here, you already do this with the getOrElse
app/models/Scheduler.scala
Outdated
val buckets = toto.bucketize(orders, 50) | ||
def handleChecks(services: List[CompleteService], tick: Tick) = { | ||
val orders = services.flatMap(service => or.getOrders(service, tick.kind)) | ||
val buckets = SchedulerInstances.bucketize(orders, 50) |
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.
This 50 number worked fine with HTTP only. Now though this should be the interval between each tick for that kind of check, minus a few seconds.
app/models/Scheduler.scala
Outdated
object SchedulerInstances { |
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.
That name does not make much sense here, I guess the bucketize function should just go in the Scheduler companion object.
.map({ case (s, vs) => CompleteService(s, vs.map(_._2))}) | ||
.toList | ||
} | ||
|
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.
Not sure why you moved this block? Not that it matters much, it just complicates the diff and makes git blaming a bit harder.
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.
The formatter was excited. I will remove this changes.
f414c38
to
5a43ec5
Compare
83de91f
to
3010622
Compare
No description provided.