openwhisk-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [openwhisk] rabbah commented on a change in pull request #4624: Combines active ack and slot release when both are available.
Date Tue, 17 Sep 2019 15:13:13 GMT
rabbah commented on a change in pull request #4624: Combines active ack and slot release when
both are available.
URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325227333
 
 

 ##########
 File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
 ##########
 @@ -112,49 +119,51 @@ object CompletionMessage extends DefaultJsonProtocol {
 case class ResultMessage(override val transid: TransactionId, response: Either[ActivationId,
WhiskActivation])
     extends AcknowledegmentMessage(transid) {
 
-  override def toString = {
-    response.fold(l => l, r => r.activationId).asString
-  }
+  override def toJson: JsValue = ResultMessage.serdes.write(this)
+  override def toString = response.fold(identity, _.activationId).asString
 }
 
-object ResultMessage extends DefaultJsonProtocol {
-  implicit def eitherResponse =
-    new JsonFormat[Either[ActivationId, WhiskActivation]] {
-      def write(either: Either[ActivationId, WhiskActivation]) = either match {
-        case Right(a) => a.toJson
-        case Left(b)  => b.toJson
-      }
+object ActivationMessage extends DefaultJsonProtocol {
+  def parse(msg: String) = Try(serdes.read(msg.parseJson))
 
-      def read(value: JsValue) = value match {
-        // per the ActivationId's serializer, it is guaranteed to be a String even if it
only consists of digits
-        case _: JsString => Left(value.convertTo[ActivationId])
-        case _: JsObject => Right(value.convertTo[WhiskActivation])
-        case _           => deserializationError("could not read ResultMessage")
-      }
-    }
+  private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
+  implicit val serdes = jsonFormat11(ActivationMessage.apply)
+}
 
 Review comment:
   Are you referring to the Objects? I found the interleaving problematic and found unnecessary
duplication (which I removed in this patch). I suspect the duplication was caused by not actually
seeing that the code was very similar because of intervening functions. Now the objects are
much more compact. So I like the current flow better of cases classes, then the singletons.
   
   If you change the type of the activation message into something incompatible the code will
not compile. `jsonFormatN` is an auto generated serdes and so if the compiler doesn't know
what to do, it will complain.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message