#802: Adding some log messages in hyperdrive trigger

[filiphornak]

I added some logging messages for the hyperdrive trigger. It will give us insight into what's happening with skipping specific workflows.

[jozefbakus]

Why not private ?

[filiphornak]

I actually don't know, but when I ran it locally, it complained that the mockito framework could not mock private static classes. I used the same command as in the GitHub actions script.

[filiphornak]

So I reverted it and will test it in the pipeline

[jozefbakus]

I don't like it, I see this idea. But I would leave as it was. It is visible that what is returned and what is a side effect on first scan

[filiphornak]

Ok, I will remove it. But, IMO, this looks clearer when you see that there is no change in value, just execution of side-effect.

Akka had something similar in their stream. Even native scala contains .tap, or .tapEach. And it could be easily implemented on any Functor type with cats.

import cats._

implicit class FunctorOps[F[_], A](val fa: F[A]) extends AnyVal {
  def tap[A, U](f: A => U)(implicit func: Functor[F]): F[A] =
    func.map(fa) { elm =>
      f(elm)
      elm
    }
}

Because, to me, this looks much cleaner

schedulerInstanceId match {
  case Some(id) => Future.successful(id)
  case _        => 
    schedulerInstanceService
      .registerNewInstance()
      .tap(id => schedulerInstanceId = Some(id))
      .omComplete {
        case Success(id) =>
          logger.info(s"Successfully assigned new (SchedulerId=${id}) to scheduler")
        case Failure(e) =>
          logger.error("Failed to get new SchedulerId", e)
      }
}

[jozefbakus]

You can rewrite this and have logging after each "flatmap"
_ = logger.trace("bla")

[filiphornak]

Yeah, I know about it, and I will change it back. However, I don't like it because the = is for a map if scala had something for specifying side effects in for comprehension.

I would instead use something like https://typelevel.org/cats/typeclasses/arrow.html or a better example with precisely the same thing https://medium.com/virtuslab/arrows-monads-and-kleisli-part-ii-12ffd4da8bc9.

To me, writing it that way looks much cleaner, and also, using cats would add several additional structures, like IO, Functor (which in turn can be used to simplify all the .map(_.map(..._.map(f)...)) operations), Validated, Writer, etc...

[jozefbakus]

JobInstance instead of JobId

[jozefbakus]

The same bellow

[jozefbakus]

Unnecessary job id twice

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication