[GitHub] [incubator-livy] ksumit commented on a diff in pull request #389: [LIVY-970][SERVER] Ordering and pagination support in GET /statement

["ksumit (via GitHub)" <gi...@apache.org>]

ksumit commented on code in PR #389:
URL: https://github.com/apache/incubator-livy/pull/389#discussion_r1132689120


##########
docs/rest-api.md:
##########
@@ -251,6 +251,27 @@ Gets the log lines from this session.
 
 Returns all the statements in a session.
 
+#### Request Parameters
+
+<table class="table">
+  <tr><th>Name</th><th>Description</th><th>Type</th></tr>
+  <tr>
+    <td>from</td>
+    <td>The start index to fetch sessions</td>
+    <td>int</td>
+  </tr>
+  <tr>
+    <td>size</td>
+    <td>Number of sessions to fetch</td>
+    <td>int</td>
+  </tr>
+  <tr>
+    <td>order</td>
+    <td>Provide value as "latest" to get latest statement first </td>

Review Comment:
   in sql world, ordering has two choices - `asc` or `desc`
   
   should we do something similar here by providing explicit choices with the default being the ordering behavior as it is right now?



##########
server/src/main/scala/org/apache/livy/server/interactive/InteractiveSessionServlet.scala:
##########
@@ -113,7 +113,12 @@ class InteractiveSessionServlet(
 
   get("/:id/statements") {
     withViewAccessSession { session =>
-      val statements = session.statements
+      val order = params.get("order")
+      val statements = if (order.map(_.trim).exists(_.equalsIgnoreCase("latest"))) {
+        session.statements.reverse

Review Comment:
   should these parameters be passed to `ReplDriver` instead and the processing be done there instead? that will be consistent to how we are handling other parameters `from` and `size`, thoughts?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org