Fix Scala evaluation command

[kaffein]

hey guys,

Sorry that it took so long to deliver this one but I had some fun (and struggles ^^) learning some new stuff along the way while implementing it ;-)...
by the way, thanks a lot @devth for helping me out when I was stuck : you provided valuable insights ... I owe you a beer (or two hehe)

So, what do we have here :

• First of all, instead of using the now deprecated scalakata API, we now target the new scastie Scala evaluation API which uses Server-Sent Events (SSE) for sending back the results.

• A small utilitiy lib, [io.nervous/kvlt "0.1.4"], has then been introduced to handle these SSEs. kvlt provides an abstraction based on core.async channels for handing out the incoming events from the server.

• There are some parameters related to the evaluation API (api endpoint url, scala-version etc.) that have been hardcoded. Should I have these parameters set in the config file or is it okay to leave them here? and in the former case, do I have to make a PR to yetibot.core for the new set of parameters then?

Here are some screenshots of how it renders so far :

Evaluation success with output

Evaluation success without output (no explicit println)

Evaluation errors

There are some stuff that we could improve on :

• It can be cool to have some clojure.specs validating the data we send/retrieve from the server. I have seen that there are some good stuff that have been implemented on karma (good job @jcorrado ^^, I will shamelessly take some inspiration from what you've done ;-)). I will probably add them from another branch as a refinement.

• We will have to fix the tests as well : they have been disabled since the API deprecation if I'm not wrong. I would like to work on it but since there is already a related issue Fixup the Scala test case #612 , I thought it would be better to work on it from another branch as well.

anyway, feel free to check out this branch and play around with it guys and as usual, do not hesitate to ping me for any suggestions/improvements that you want me to add to this patch.

thanks a lot...

[devth]

Awesome work @kaffein!! I’ll try to check this out later today.

[devth]

Hmm, when I try to load the yetibot.commands.scala namespace in my REPL it throws an exception:

|| java.lang.Exception: namespace 'aleph.http.client-middleware' not found, compiling:(aleph/http/client.clj:1:1)
zipfile:/Users/thartman/.m2/repository/org/clojure/clojure/1.9.0/clojure-1.9.0.jar::clojure/core.clj|5807| clojure.core$throw_if.invokeStatic
zipfile:/Users/thartman/.m2/repository/org/clojure/clojure/1.9.0/clojure-1.9.0.jar::clojure/core.clj|5893| clojure.core$load_lib.invokeStatic
zipfile:/Users/thartman/.m2/repository/org/clojure/clojure/1.9.0/clojure-1.9.0.jar::clojure/core.clj|5868| clojure.core$load_lib.doInvoke
|| clojure.lang.RestFn.applyTo(RestFn.java:142)

If I lein run instead I see in the logs:

java.lang.ClassNotFoundException: javax.xml.bind.DatatypeConverter, compiling:(aleph/http/client_middleware.clj:407:7)
java.lang.ClassNotFoundException: javax.xml.bind.DatatypeConverter
 at java.net.URLClassLoader.findClass (URLClassLoader.java:466)
    clojure.lang.DynamicClassLoader.findClass (DynamicClassLoader.java:69)
    java.lang.ClassLoader.loadClass (ClassLoader.java:566)
    clojure.lang.DynamicClassLoader.loadClass (DynamicClassLoader.java:77)
    java.lang.ClassLoader.loadClass (ClassLoader.java:499)
    java.lang.Class.forName0 (Class.java:-2)
    java.lang.Class.forName (Class.java:374)
    clojure.lang.RT.classForName (RT.java:2204)
    clojure.lang.RT.classForNameNonLoading (RT.java:2217)
    clojure.lang.Compiler$HostExpr.maybeClass (Compiler.java:1041)
    clojure.lang.Compiler.macroexpand1 (Compiler.java:6942)
    clojure.lang.Compiler.analyzeSeq (Compiler.java:6989)
    clojure.lang.Compiler.analyze (Compiler.java:6773)
    clojure.lang.Compiler.analyzeSeq (Compiler.java:6991)
    clojure.lang.Compiler.analyze (Compiler.java:6773)

Java version:

$ java -version
java version "10.0.2" 2018-07-17
Java(TM) SE Runtime Environment 18.3 (build 10.0.2+13)
Java HotSpot(TM) 64-Bit Server VM 18.3 (build 10.0.2+13, mixed mode)

Which version of java are you on?

Also I noticed kvlt is an archived repo. Kinda strange. Was that recent? I see it uses an outdated version of aleph. Maybe we could exclude it and force using the latest version.

[kaffein]

that's weird indeed!

I'm still on java 1.8 here :

java -version
java version "1.8.0_112"
Java(TM) SE Runtime Environment (build 1.8.0_112-b16)
Java HotSpot(TM) 64-Bit Server VM (build 25.112-b16, mixed mode)

Should I upgrade java as well ?

we can try forcing it to use the most recent version of aleph as you said ...

[devth]

Don’t worry about upgrading unless you want to 😄. I sort of accidentally ended up on Java 10. Planning to upgrade to 11 once clojure 1.10 comes out. I can try forcing aleph on my side.

[devth]

@kaffein upgrading aleph fixed the issue!!

If you add this to project.clj below the kvlt dep I'll retest and approve:

                 ;; overwrite kvlt's outdated version of aleph
                 [aleph "0.4.6"]

[kaffein]

awesome @devth thanks ... I will add it to the project.clj and will let you know

[kaffein]

There it is @devth
Can you PTAL one last time to make sure I didn't screw things up ?

again thanks

[devth]

It works!!! Thanks @kaffein. This is an epic PR!

[devth]

💯 💥

[devth]

And to answer your question:

Should I have these parameters set in the config file or is it okay to leave them here? and in the former case, do I have to make a PR to yetibot.core for the new set of parameters then?

Let's leave them hardcoded. If there's ever a need we can extract them out. The only change to yetibot.core would be in the config examples (basically documentation).

[kaffein]

Wouhouuuu \m/
thanks a lot @devth ;-)