gravitydev / traction

3 stars 3 forks source link

Clarification of handling of Either + Conditional steps #3

Open dspasojevic opened 10 years ago

dspasojevic commented 10 years ago

Hi,

I am looking at using traction to handle some of our SWF workload. I've been doing some testing to see what is possible, and have a couple of questions:

  1. Does pickleSerializer support Either and Option? I don't seem to be able to get this to work. For example, in the console:
scala> import scala.pickling._, json._
import scala.pickling._
import json._

scala> import com.gravitydev.traction._
import com.gravitydev.traction._

scala> val s = pickleSerializer[Either[String, String]]
s: com.gravitydev.traction.PickleSerializer[Either[String,String]] = com.gravitydev.traction.PickleSerializer@b290aed

scala> s.serialize(Left("bob"))
scala.NotImplementedError: an implementation is missing
  at scala.Predef$.$qmark$qmark$qmark(Predef.scala:225)
  at scala.pickling.PicklerUnpicklerNotFound.pickle(Custom.scala:22)
  at com.gravitydev.traction.PickleSerializer.serialize(package.scala:7)
  ... 43 elided

I see the same failure if I change Sample to pass around Eithers (or Options).

  1. Conditional Steps. I would like to be able to run different activities depending on the result of the previous activities. At the moment I am doing this by manually converting the Activity into a Step:
case class DoSomethingrecord: Record) extends Workflow[Record] {

  import model.Workflow._

  def flow: Step[Record] = {

    def complete(record: Record): Step[Record] = toStep1(UpdateRecord(record, "completed", None))(updateRecordA).map {
      _ match {
        case MFResult(_, Right(completedRecord)) => completedRecord
        case _ => Record("", "", "", None)
      }
    }

    def markFailed(record: Record): Step[Record] = toStep1(UpdateRecord(record, "failed", None))(updateRecordA).map {
      _ match {
        case MFResult(_, Right(failedRecord)) => failedRecord
        case _ => Record("", "", "", None)
      }
    }

    val starter: Step[Record] = toStep1(ProcessTheRecord(record))(processRecordA).flatMap( result => {
      result match {
        case MFResult(_, Right(processedRecord)) => complete(processedRecord)
        case MFResult(_, Left(error)) => {
          Console.out.println(s"Failed to update record: [$error]")
          markFailed(record)
        }
      }
    })

    starter
  }
}

This appears to do the correct thing, but I am wondering if there is there a nicer way of expressing that conditional workflow? I feel like I am missing something about how to support this. Do you think that I would be better off implementing a ConditionalStep as a peer to MappedStep, etc?

Thanks, -Dan

alvaroc1 commented 10 years ago

Hi @dspasojevic A: 1. You are right, I hadn't tested Options and Eithers. At the moment, I think you would have to specify a separate instance for each serializer:

val leftS = pickleSerializer[Left[String, String]]
val rightS = pickleSerializer[Right[String, String]]

Not pretty, I know. I'll look into it and see if we can provide a better API for serializing those.

A: 2. I do think that Conditional steps should be peers to MappedStep. I was planning on building that, but haven't had time. I am not sure what the API should be yet. But the idea is that the entire workflow would always read nicely on a for-comprehension.

Let me know if that helps, I may get some time to work on these later in the week.

alvaroc1 commented 10 years ago

@dspasojevic On the "upickle" branch, I switched to uPickle for the serialization. It seems to be much more capable and easier to work with. You might want to try that branch or here's the artifact:

"com.gravitydev" %% "traction-amazonswf" % "0.1.1-upickle-SNAPSHOT"

I'll merge that to master if things look good later in the week.

dspasojevic commented 10 years ago

Thanks @alvaroc1, for me at least, that change works great.

dspasojevic commented 10 years ago

Hi,

It looks like the more recent versions of upickle (I've tried 0.2.2 and 0.2.4) do not handle having a Iterable (or a case class that contains an Iterable) as a constructor argument to an activity. The same case class works as expected when used as a constructor argument to a workflow.

I've reverted to 0.1.7 and - with a bit of fooling around to get the implicits to the right spot - things work as I expected.

Thanks again.