You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@livy.apache.org by va...@apache.org on 2018/11/05 17:36:18 UTC

incubator-livy git commit: [LIVY-525] SessionServlet: Convert response body for error messages to json

Repository: incubator-livy
Updated Branches:
  refs/heads/master 0442eb01a -> 0bcc290dd


[LIVY-525] SessionServlet: Convert response body for error messages to json

## What changes were proposed in this pull request?
SessionServlet is supposed to deliver json responses. This is not the case in some error cases today. This pull requests converts the response bodies for NotFound, BadRequest and Forbidden to json
https://issues.apache.org/jira/browse/LIVY-525

## How was this patch tested?
manual tests since this is a trivial fix.

Author: ingo <in...@de.ibm.com>

Closes #127 from IngoSchuster/json_fix.


Project: http://git-wip-us.apache.org/repos/asf/incubator-livy/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-livy/commit/0bcc290d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-livy/tree/0bcc290d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-livy/diff/0bcc290d

Branch: refs/heads/master
Commit: 0bcc290dd2bccef85645f1a35938c241974b5c17
Parents: 0442eb0
Author: ingo <in...@de.ibm.com>
Authored: Mon Nov 5 09:36:12 2018 -0800
Committer: Marcelo Vanzin <va...@cloudera.com>
Committed: Mon Nov 5 09:36:12 2018 -0800

----------------------------------------------------------------------
 .../main/scala/org/apache/livy/server/JsonServlet.scala |  4 ++++
 .../scala/org/apache/livy/server/SessionServlet.scala   | 12 ++++++------
 .../scala/org/apache/livy/server/JsonServletSpec.scala  | 10 ++++++++++
 3 files changed, 20 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/0bcc290d/server/src/main/scala/org/apache/livy/server/JsonServlet.scala
----------------------------------------------------------------------
diff --git a/server/src/main/scala/org/apache/livy/server/JsonServlet.scala b/server/src/main/scala/org/apache/livy/server/JsonServlet.scala
index c1db9aa..25e1989 100644
--- a/server/src/main/scala/org/apache/livy/server/JsonServlet.scala
+++ b/server/src/main/scala/org/apache/livy/server/JsonServlet.scala
@@ -40,6 +40,8 @@ abstract class JsonServlet extends ScalatraServlet with ApiFormats with FutureSu
 
   override protected implicit def executor: ExecutionContext = ExecutionContext.global
 
+  case class ResponseMessage(msg: String)
+
   private lazy val _defaultMapper = new ObjectMapper()
     .registerModule(com.fasterxml.jackson.module.scala.DefaultScalaModule)
 
@@ -84,6 +86,8 @@ abstract class JsonServlet extends ScalatraServlet with ApiFormats with FutureSu
 
   override protected def renderResponseBody(actionResult: Any): Unit = {
     val result = actionResult match {
+      case ActionResult(status, ResponseMessage(msg), headers) if format == "json" =>
+        ActionResult(status, toJson(Map("msg" -> msg)), headers)
       case ActionResult(status, body, headers) if format == "json" =>
         ActionResult(status, toJson(body), headers)
       case str: String if format == "json" =>

http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/0bcc290d/server/src/main/scala/org/apache/livy/server/SessionServlet.scala
----------------------------------------------------------------------
diff --git a/server/src/main/scala/org/apache/livy/server/SessionServlet.scala b/server/src/main/scala/org/apache/livy/server/SessionServlet.scala
index 928aa05..f7547c5 100644
--- a/server/src/main/scala/org/apache/livy/server/SessionServlet.scala
+++ b/server/src/main/scala/org/apache/livy/server/SessionServlet.scala
@@ -112,10 +112,10 @@ abstract class SessionServlet[S <: Session, R <: RecoveryMetadata](
       sessionManager.delete(session.id) match {
         case Some(future) =>
           Await.ready(future, Duration.Inf)
-          Ok(Map("msg" -> "deleted"))
+          Ok(ResponseMessage("deleted"))
 
         case None =>
-          NotFound(s"Session ${session.id} already stopped.")
+          NotFound(ResponseMessage(s"Session ${session.id} already stopped."))
       }
     }
   }
@@ -128,7 +128,7 @@ abstract class SessionServlet[S <: Session, R <: RecoveryMetadata](
 
   post("/") {
     if (tooManySessions) {
-      BadRequest("Rejected, too many sessions are being created!")
+      BadRequest(ResponseMessage("Rejected, too many sessions are being created!"))
     } else {
       val session = sessionManager.register(createSession(request))
       // Because it may take some time to establish the session, update the last activity
@@ -149,8 +149,8 @@ abstract class SessionServlet[S <: Session, R <: RecoveryMetadata](
   }
 
   error {
-    case e: IllegalArgumentException => BadRequest(e.getMessage)
-    case e: AccessControlException => Forbidden(e.getMessage)
+    case e: IllegalArgumentException => BadRequest(ResponseMessage(e.getMessage))
+    case e: AccessControlException => Forbidden(ResponseMessage(e.getMessage))
   }
 
   /**
@@ -191,7 +191,7 @@ abstract class SessionServlet[S <: Session, R <: RecoveryMetadata](
           Forbidden()
         }
       case None =>
-        NotFound(s"Session '$sessionId' not found.")
+        NotFound(ResponseMessage(s"Session '$sessionId' not found."))
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/0bcc290d/server/src/test/scala/org/apache/livy/server/JsonServletSpec.scala
----------------------------------------------------------------------
diff --git a/server/src/test/scala/org/apache/livy/server/JsonServletSpec.scala b/server/src/test/scala/org/apache/livy/server/JsonServletSpec.scala
index 5ca3997..dcf1a3f 100644
--- a/server/src/test/scala/org/apache/livy/server/JsonServletSpec.scala
+++ b/server/src/test/scala/org/apache/livy/server/JsonServletSpec.scala
@@ -89,6 +89,13 @@ class JsonServletSpec extends BaseJsonServletSpec {
       }
     }
 
+    it("should send (error) response message as json") {
+      get("/responsemessage") {
+        assert(status === SC_NOT_FOUND)
+        assert(response.body === "{\"msg\":\"response message\"}" )
+      }
+    }
+
   }
 
 }
@@ -145,5 +152,8 @@ private class TestJsonServlet extends JsonServlet {
       ServiceUnavailable(e.getMessage())
   }
 
+  get("/responsemessage") {
+    NotFound(ResponseMessage("response message"))
+  }
 }