Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DL4J UI: Migrate from Play to Vertx #68

Merged
merged 25 commits into from Nov 22, 2019
Merged

DL4J UI: Migrate from Play to Vertx #68

merged 25 commits into from Nov 22, 2019

Conversation

@AlexDBlack
Copy link
Member

AlexDBlack commented Nov 21, 2019

DL4J and Arbiter UIs done, tested and working.
No API changes for users, should be seamless - existing UI examples run OK without any modification.

Note that unlike Play, Vertx doesn't rely on Scala - so the module names have changed:
deeplearning4j-ui_2.1x -> deeplearning4j-ui
deeplearning4j-play_2.1x -> deeplearning4j-vertx
(Note that users should still just import deeplearning4j-ui as before)

Fixes: eclipse#8413

AlexDBlack added 20 commits Nov 19, 2019
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Copy link

agibsonccc left a comment

Good so far!

log.debug("Arbiter UI: Set to session {}", newSessionID);

if (knownSessionIDs.containsKey(newSessionID)) {
currentSessionID = newSessionID;
return ok();
rc.response().end();

This comment has been minimized.

Copy link
@agibsonccc

agibsonccc Nov 22, 2019

Returning a status object might not be a bad idea here.

This comment has been minimized.

Copy link
@AlexDBlack

AlexDBlack Nov 22, 2019

Author Member

You mean like .setStatusCode(HttpResponseStatus.OK.code())?
It's 200/ok by default... https://vertx.io/blog/some-rest-with-vert-x/

@@ -41,99 +38,81 @@
* @return UI instance for this JVM
* @throws RuntimeException if the instance has already started in a different mode (multi/single-session)
*/
public static synchronized UIServer getInstance() throws RuntimeException {
static UIServer getInstance() throws RuntimeException {

This comment has been minimized.

Copy link
@agibsonccc

agibsonccc Nov 22, 2019

Any reason we got rid of the public declaration?

This comment has been minimized.

Copy link
@AlexDBlack

AlexDBlack Nov 22, 2019

Author Member

UIServer used to be an abstract class, now it's an interface (with java 8 static interface methods for consistency with the beta5 and earlier API). So it's public by definition.
Mainly because VertxUIServer class needs to extend AbstractVerticle for Vertx.
It's same API for users, they can still call UIServer.getInstance() as in beta5 and earlier.

import java.util.Arrays;

@Ignore
public class TestSameDiffUI {

This comment has been minimized.

Copy link
@agibsonccc

This comment has been minimized.

Copy link
@AlexDBlack

AlexDBlack Nov 22, 2019

Author Member

Cleaned up paths now. But this is otherwise just transferred over from master.
Overall SameDiff UI is WIP/POC stage, I've mainly been using it for debugging (to visualize complex graph structures). It works but it's by no means production ready.

@AlexDBlack AlexDBlack force-pushed the ab_vertx branch from c06627f to f58349e Nov 22, 2019
AlexDBlack added 5 commits Nov 22, 2019
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
Signed-off-by: AlexDBlack <blacka101@gmail.com>
@AlexDBlack AlexDBlack marked this pull request as ready for review Nov 22, 2019
@AlexDBlack AlexDBlack merged commit 6f514e9 into master Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.