Add Akka HTTP server project #55

kellyi · 2017-07-31T20:58:35Z

Overview

This PR adds an Akka HTTP server to the repo as a separate project. The server lives in api/src/main/scala/WebServer.scala and it's currently got two endpoints:

GET :8090/ping which returns pong
POST :8090/run which accepts a string, prints it to the log, then returns a hard coded heredoc-type string representing the expected output.

I also added two short scripts: build, which runs ./sbt assembly and debugserver which runs ./sbt ~re-start.

The latter watches the file system for changes & recompiles when it sees them and it should shorten the development loop, since we'll no longer have to create a jar file to test each change. However, it currently only works with the WebServer because it expects there to be some sort of Main class in each project -- and we don't currently have one in the summary dir.

One solution to this would be to just put the server in the same dir and the job files be siblings. Since there's only a single file for the server, it wouldn't clutter up the namespace too much.

Connects #48
Connects #50

Testing

get this branch, then run ./sbt compile
run ./scripts/debugserver and verify that the server starts
http POST :8090/run hello=world and verify that you: a) get the hard-coded output back and b) can see {hello: "world"} in the server log
http :8090/ping and verify that you see a pong
revise the ping endpoint such that it concludes with complete("hello world"), then save the file. Verify that the project's recompiled and that you can http :8090/ping to see the new output

kellyi · 2017-07-31T21:00:02Z

We'll need to update the README at some point, but I'm not sure what that point is. I'm happy to make some small changes here if we'd like.

rajadain · 2017-07-31T21:02:29Z

I made #56 for updating the README. I think we should do it after the dust has settled.

kellyi · 2017-07-31T21:06:01Z

Going to add a commit to incorporate some of the work on this branch:

https://github.com/WikiWatershed/mmw-geoprocessing/blob/tt%2Bec/collections-api/api/src/main/scala/Router.scala

kellyi · 2017-07-31T21:39:14Z

api/src/main/scala/WebServer.scala

@@ -0,0 +1,134 @@
+package org.wikiwatershed.geoprocessing


Oops, there's a typo in the package name.

hectcastro · 2017-07-31T21:14:20Z

project/build.properties

@@ -0,0 +1 @@
+sbt.version=0.13.11


I believe that 0.13.16 exists now.

hectcastro · 2017-08-01T11:44:18Z

api/src/main/scala/WebServer.scala

+object Main {
+  implicit val system = ActorSystem("mmw-geoprocessing")
+  implicit val materializer = ActorMaterializer()
+  protected implicit val log = Logging(system, "app")


From what I can tell, we've been trying to standardize on https://github.com/typesafehub/scala-logging by making use of the LazyLogging trait to make a logger val available within classes. It would be good to try and use that approach again here vs. the lower level Logging instance.

hectcastro · 2017-08-01T11:46:09Z

project/build.scala

@@ -5,17 +5,22 @@ import scala.util.Properties
 // sbt-assembly


This may not be the place to do it, but it would be great to replace this dated build.scala with a simpler build.sbt.

Going to look into doing this here -- sbt 0.13.16 shows a warning about using build.scala, but the project still compiles. If moving to build.sbtturns out to be really involved I'll spin it off into another card.

Made #55 to handle these changes.

hectcastro · 2017-08-01T11:46:36Z

project/build.scala

+        "com.typesafe.akka" %% "akka-http" % Version.akkaHttpVersion,
+        "com.typesafe.akka" %% "akka-stream" % Version.akkaVersion,
+        "com.typesafe.akka" %% "akka-http-spray-json" % Version.akkaHttpVersion,
+        "ch.megard" %% "akka-http-cors" % Version.akkaHttpCorsVersion,


Do we actually need CORS support for this endpoint?

No we should not, since the service is only ever accessed locally.

Ah, good point. I'll drop this!

hectcastro · 2017-08-01T11:47:36Z

scripts/build

@@ -0,0 +1,4 @@
+#!/usr/bin/env bash


Any reason not to rename this file to cibuild? It seems like the assembly JAR is what we'd want out of a cibuild process (in addition to the execution of some tests).

Nope! I'll rename it.

hectcastro · 2017-08-01T11:47:50Z

scripts/debugserver

@@ -0,0 +1,4 @@
+#!/usr/bin/env bash


Any reason not to rename this to server?

Will rename this one, too.

hectcastro · 2017-08-01T11:48:07Z

api/src/main/resources/application.conf

@@ -0,0 +1,4 @@
+geoprocessing {


Good thought to start this configuration file early.

hectcastro · 2017-08-01T11:52:14Z

api/src/main/scala/Router.scala

+
+import com.typesafe.config.ConfigFactory
+
+class Router extends Directives {


Instead of breaking out the router early, does it make sense to simplify things here so that we extend from HttpApp so that we get things like an ActorSystem, graceful termination, and a method for overriding routes for free?

I didn't know about HttpApp, here's some documentation http://doc.akka.io/docs/akka-http/10.0.9/scala/http/routing-dsl/HttpApp.html

rajadain

This is a good start. Works well, except for the text/plain response like I've indicated. I wonder if we should have someone from another team that has more experience with Akka also take a look. Also could use some Scala expertise with build.sbt.

rajadain · 2017-08-01T14:00:05Z

scripts/build

+#!/usr/bin/env bash
+set -ex
+
+./sbt assembly


This should probably be scoped to the api project:

./sbt "project api" assembly

rajadain · 2017-08-01T14:00:22Z

scripts/debugserver

+#!/usr/bin/env bash
+set -ex
+
+./sbt ~re-start


Same here:

./sbt "project api" ~re-start

rajadain · 2017-08-01T14:21:36Z

scripts/cibuild

+#!/usr/bin/env bash
+set -ex
+
+./sbt assembly


Consider scoping to the api project

./sbt "project api" assembly

rajadain · 2017-08-01T14:22:11Z

scripts/server

+#!/usr/bin/env bash
+set -ex
+
+./sbt ~re-start


Similarly

./sbt "project api" ~re-start

rajadain · 2017-08-01T14:26:45Z

api/src/main/scala/Router.scala

+          println(input)
+          val output =
+          """
+          {


While we do want to be minimal, we also want this to be a good template to build the Celery tasks against. The current implementation returns Content-Type: text/plain instead of application/json. To return application/json, do this:

diff --git a/api/src/main/scala/Router.scala b/api/src/main/scala/Router.scala index 716b5c2..435a259 100644 --- a/api/src/main/scala/Router.scala +++ b/api/src/main/scala/Router.scala @@ -5,6 +5,7 @@ import akka.http.scaladsl.model.HttpEntity import akka.http.scaladsl.server.Directives import akka.http.scaladsl.unmarshalling.Unmarshaller._ import spray.json._ +import spray.json.DefaultJsonProtocol._ import ch.megard.akka.http.cors.scaladsl.CorsDirectives._ import scala.collection.JavaConversions._ @@ -25,102 +26,98 @@ class Router extends Directives { path("run") { entity(as[String]) { input => println(input) - val output = - """ - { - "result": { - "List(90, 1)": 1, - "List(31, 3)": 57, - "List(81, 7)": 514, - "List(52, 7)": 416, - "List(43, 7)": 36, - "List(21, 3)": 8134, - "List(43, 2)": 46, - "List(23, 2)": 670, - "List(24, 1)": 62, - "List(23, 1)": 512, - "List(23, 7)": 311, - "List(41, 1)": 50, - "List(21, 6)": 176, - "List(21, -2147483648)": 16027, - "List(81, 3)": 2647, - "List(31, 2)": 21, - "List(71, 4)": 126, - "List(42, 3)": 72, - "List(52, 1)": 13, - "List(43, 4)": 50, - "List(31, 4)": 165, - "List(71, -2147483648)": 72, - "List(22, 7)": 969, - "List(22, -2147483648)": 16279, - "List(31, 7)": 58, - "List(24, 7)": 33, - "List(22, 1)": 603, - "List(81, 6)": 10, - "List(82, 4)": 2133, - "List(41, 4)": 5379, - "List(82, -2147483648)": 268, - "List(22, 2)": 1636, - "List(21, 1)": 1601, - "List(81, 2)": 1956, - "List(90, 6)": 19, - "List(41, 2)": 3008, - "List(41, 7)": 4232, - "List(81, 1)": 28, - "List(95, 3)": 14, - "List(23, 6)": 10, - "List(82, 3)": 1889, - "List(42, 2)": 13, - "List(21, 4)": 6982, - "List(43, -2147483648)": 106, - "List(52, 4)": 971, - "List(82, 7)": 306, - "List(90, 4)": 509, - "List(95, 4)": 27, - "List(21, 7)": 3241, - "List(81, -2147483648)": 1086, - "List(52, -2147483648)": 585, - "List(71, 6)": 7, - "List(11, 1)": 2, - "List(71, 2)": 157, - "List(90, -2147483648)": 399, - "List(11, -2147483648)": 32, - "List(41, 3)": 4419, - "List(24, 3)": 372, - "List(42, 4)": 43, - "List(11, 4)": 5, - "List(95, 7)": 20, - "List(22, 4)": 2876, - "List(90, 7)": 2500, - "List(24, 4)": 100, - "List(41, -2147483648)": 2068, - "List(82, 2)": 1716, - "List(52, 3)": 960, - "List(42, -2147483648)": 25, - "List(95, 2)": 2, - "List(90, 3)": 404, - "List(52, 2)": 357, - "List(22, 6)": 47, - "List(31, -2147483648)": 63, - "List(95, -2147483648)": 49, - "List(23, 3)": 1188, - "List(23, -2147483648)": 7223, - "List(41, 6)": 62, - "List(24, -2147483648)": 3148, - "List(24, 2)": 78, - "List(21, 2)": 4397, - "List(22, 3)": 2820, - "List(52, 6)": 7, - "List(90, 2)": 108, - "List(43, 3)": 91, - "List(71, 7)": 101, - "List(81, 4)": 2681, - "List(71, 3)": 221, - "List(23, 4)": 1062, - "List(82, 1)": 33 - } - } - """ + val NODATA = Int.MinValue + val output = Map( + List(11, NODATA) -> 32, + List(11, 1) -> 2, + List(11, 4) -> 5, + List(21, NODATA) -> 16027, + List(21, 1) -> 1601, + List(21, 2) -> 4397, + List(21, 3) -> 8134, + List(21, 4) -> 6982, + List(21, 6) -> 176, + List(21, 7) -> 3241, + List(22, NODATA) -> 16279, + List(22, 1) -> 603, + List(22, 2) -> 1636, + List(22, 3) -> 2820, + List(22, 4) -> 2876, + List(22, 6) -> 47, + List(22, 7) -> 969, + List(23, NODATA) -> 7223, + List(23, 1) -> 512, + List(23, 2) -> 670, + List(23, 3) -> 1188, + List(23, 4) -> 1062, + List(23, 6) -> 10, + List(23, 7) -> 311, + List(24, NODATA) -> 3148, + List(24, 1) -> 62, + List(24, 2) -> 78, + List(24, 3) -> 372, + List(24, 4) -> 100, + List(24, 7) -> 33, + List(31, NODATA) -> 63, + List(31, 2) -> 21, + List(31, 3) -> 57, + List(31, 4) -> 165, + List(31, 7) -> 58, + List(41, NODATA) -> 2068, + List(41, 1) -> 50, + List(41, 2) -> 3008, + List(41, 3) -> 4419, + List(41, 4) -> 5379, + List(41, 6) -> 62, + List(41, 7) -> 4232, + List(42, NODATA) -> 25, + List(42, 2) -> 13, + List(42, 3) -> 72, + List(42, 4) -> 43, + List(43, NODATA) -> 106, + List(43, 2) -> 46, + List(43, 3) -> 91, + List(43, 4) -> 50, + List(43, 7) -> 36, + List(52, NODATA) -> 585, + List(52, 1) -> 13, + List(52, 2) -> 357, + List(52, 3) -> 960, + List(52, 4) -> 971, + List(52, 6) -> 7, + List(52, 7) -> 416, + List(71, NODATA) -> 72, + List(71, 2) -> 157, + List(71, 3) -> 221, + List(71, 4) -> 126, + List(71, 6) -> 7, + List(71, 7) -> 101, + List(81, NODATA) -> 1086, + List(81, 1) -> 28, + List(81, 2) -> 1956, + List(81, 3) -> 2647, + List(81, 4) -> 2681, + List(81, 6) -> 10, + List(81, 7) -> 514, + List(82, NODATA) -> 268, + List(82, 1) -> 33, + List(82, 2) -> 1716, + List(82, 3) -> 1889, + List(82, 4) -> 2133, + List(82, 7) -> 306, + List(90, NODATA) -> 399, + List(90, 1) -> 1, + List(90, 2) -> 108, + List(90, 3) -> 404, + List(90, 4) -> 509, + List(90, 6) -> 19, + List(90, 7) -> 2500, + List(95, NODATA) -> 49, + List(95, 2) -> 2, + List(95, 3) -> 14, + List(95, 4) -> 27, + List(95, 7) -> 20 + ) map { case (k, v) => k.toString -> v } complete(output) } }

rajadain · 2017-08-01T14:28:01Z

api/src/main/scala/WebServer.scala

+package org.wikiwatershed.mmw.geoprocessing
+
+import akka.actor.ActorSystem
+import akka.actor.Props


Some of these imports are not used: Props and IO

rajadain · 2017-08-01T14:30:20Z

api/src/main/scala/Router.scala

+
+import com.typesafe.config.ConfigFactory
+
+class Router extends Directives {


I didn't know about HttpApp, here's some documentation http://doc.akka.io/docs/akka-http/10.0.9/scala/http/routing-dsl/HttpApp.html

rajadain · 2017-08-01T14:32:13Z

api/src/main/scala/Router.scala

+package org.wikiwatershed.mmw.geoprocessing
+
+import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
+import akka.http.scaladsl.model.HttpEntity


Some of these imports are never used. I would prefer to work minimally, and only add them when needed. Consider removing HttpEntity, spray.json._, CorsDirectives, JavaConversions, Future, ExecutionContext, and ConfigFactory.

kellyi · 2017-08-01T16:36:07Z

Pushed a few new commits which...

adjust the update and server scripts to specify "project api"
replace the prior server setup with one that uses HttpApp:
add a LazyLogging trait to the server:

object WebServer extends HttpApp with App with LazyLogging

upgrade Revolver from 0.7.2 -> 0.8.0
squash the routes and server into a single file
drop unused imports
adjust /run to serialize a Scala map to JSON for the output

I made #55 to handle updating the build.scala file to build.sbt, as it seems like this will be involved enough to merit more investigation & review

kellyi · 2017-08-01T16:37:25Z

These changes now also add setup, update, and console scripts with minimal implementations.

rajadain · 2017-08-01T16:42:08Z

Taking another look.

rajadain

+1 tested, this works well and looks really good. Nice work!

rajadain · 2017-08-01T17:09:12Z

api/src/main/scala/WebServer.scala

+import akka.http.scaladsl.unmarshalling.Unmarshaller._
+import spray.json._
+import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
+import DefaultJsonProtocol._


This import is not used.

Interesting. This seems to be required to be able to call toJson on the output below -- without it, the project won't compile on my local because

Cannot find JsonWriter or JsonFormat type class for scala.collection.immutable.Map[String,Int]

Oh. Okay let's keep it then. IntelliJ IDEA usually is right about these things, but every once in a while it'll goof.

Sounds good. I left it in. I adjusted the order of the imports here to keep akka things etc together.

- add akka-http-server with a `/ping` get endpoint and a `/run` post endpoint - adjust build.scala to build multiple projects - upgrade and enable sbt-revolver - add cibuild, console, server, setup, and update scripts - add application.conf - specify using sbt 0.13.16 - add LazyLogging trait

kellyi · 2017-08-01T17:53:20Z

Thanks for all your help with this! I squashed everything down to a single commit. Going to double check that everything still works as expected then merge.

kellyi added the in progress label Jul 31, 2017

kellyi requested a review from rajadain July 31, 2017 20:58

kellyi assigned rajadain Jul 31, 2017

kellyi requested a review from hectcastro July 31, 2017 20:58

kellyi commented Jul 31, 2017

View reviewed changes

kellyi changed the title ~~Add akka-http-server project~~ Add Akka HTTP server project Jul 31, 2017

hectcastro reviewed Aug 1, 2017

View reviewed changes

rajadain reviewed Aug 1, 2017

View reviewed changes

kellyi mentioned this pull request Aug 1, 2017

Collections API: Replace build.scala with build.sbt #57

Open

rajadain approved these changes Aug 1, 2017

View reviewed changes

rajadain assigned kellyi and unassigned rajadain Aug 1, 2017

hectcastro approved these changes Aug 1, 2017

View reviewed changes

kellyi force-pushed the ki/add-akka-http-server branch from 3b50860 to a1cb4ef Compare August 1, 2017 17:51

kellyi merged commit f893ee7 into develop Aug 1, 2017

kellyi deleted the ki/add-akka-http-server branch August 1, 2017 17:56

kellyi removed the in progress label Aug 1, 2017

		@@ -5,17 +5,22 @@ import scala.util.Properties
		// sbt-assembly


		import com.typesafe.config.ConfigFactory

		class Router extends Directives {

		@@ -0,0 +1 @@
		sbt.version=0.13.11

Add Akka HTTP server project #55

Add Akka HTTP server project #55

Conversation

kellyi commented Jul 31, 2017 • edited Loading

Overview

Testing

kellyi commented Jul 31, 2017

rajadain commented Jul 31, 2017 • edited Loading

kellyi commented Jul 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajadain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kellyi commented Aug 1, 2017

kellyi commented Aug 1, 2017

rajadain commented Aug 1, 2017

rajadain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kellyi commented Aug 1, 2017

kellyi commented Jul 31, 2017 •

edited

Loading

rajadain commented Jul 31, 2017 •

edited

Loading