From 83244be603047b8fdd9b156b959a8dd23d379935 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Fri, 10 Jun 2016 15:51:22 +0900 Subject: [PATCH 1/3] Removed remote option from InterpreterOption and related codes and tests --- .../zeppelin/rest/AbstractTestRestApi.java | 2 +- .../interpreter/InterpreterFactory.java | 52 +++++++++---------- .../interpreter/InterpreterOption.java | 26 +++++----- .../interpreter/InterpreterFactoryTest.java | 6 +-- .../notebook/NoteInterpreterLoaderTest.java | 2 +- .../zeppelin/notebook/NotebookTest.java | 2 +- .../notebook/repo/NotebookRepoSyncTest.java | 2 +- .../notebook/repo/VFSNotebookRepoTest.java | 2 +- 8 files changed, 47 insertions(+), 47 deletions(-) diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java index 5ea3c0943a5..613a2d07de0 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java @@ -378,7 +378,7 @@ protected String createTempSetting(String tempName) .add(tempName, "newGroup", new LinkedList(), - new InterpreterOption(false), + new InterpreterOption(), new Properties()); return setting.id(); } diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java index bad18c02347..a8fd4b5fe30 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java @@ -88,7 +88,7 @@ public InterpreterFactory(ZeppelinConfiguration conf, RemoteInterpreterProcessListener remoteInterpreterProcessListener, DependencyResolver depResolver) throws InterpreterException, IOException, RepositoryException { - this(conf, new InterpreterOption(true), angularObjectRegistryListener, + this(conf, new InterpreterOption(), angularObjectRegistryListener, remoteInterpreterProcessListener, depResolver); } @@ -312,7 +312,7 @@ private void loadFromFile() throws IOException { // While we decided to turn this feature on always (without providing // enable/disable option on GUI). // previously created setting should turn this feature on here. - setting.getOption().setRemote(true); +// setting.getOption().setRemote(true); InterpreterSetting intpSetting = new InterpreterSetting( setting.id(), @@ -499,19 +499,19 @@ public InterpreterGroup createInterpreterGroup(String id, InterpreterOption opti AngularObjectRegistry angularObjectRegistry; InterpreterGroup interpreterGroup = new InterpreterGroup(id); - if (option.isRemote()) { - angularObjectRegistry = new RemoteAngularObjectRegistry( - id, - angularObjectRegistryListener, - interpreterGroup - ); - } else { - angularObjectRegistry = new AngularObjectRegistry( - id, - angularObjectRegistryListener); - - // TODO(moon) : create distributed resource pool for local interpreters and set - } +// if (option.isRemote()) { + angularObjectRegistry = new RemoteAngularObjectRegistry( + id, + angularObjectRegistryListener, + interpreterGroup + ); +// } else { +// angularObjectRegistry = new AngularObjectRegistry( +// id, +// angularObjectRegistryListener); +// +// // TODO(moon) : create distributed resource pool for local interpreters and set +// } interpreterGroup.setAngularObjectRegistry(angularObjectRegistry); return interpreterGroup; @@ -581,17 +581,17 @@ public void createInterpretersForNote( && info.getGroup().equals(groupName)) { Interpreter intp; - if (option.isRemote()) { - intp = createRemoteRepl(info.getPath(), - key, - info.getClassName(), - properties, - interpreterSetting.id()); - } else { - intp = createRepl(info.getPath(), - info.getClassName(), - properties); - } +// if (option.isRemote()) { + intp = createRemoteRepl(info.getPath(), + key, + info.getClassName(), + properties, + interpreterSetting.id()); +// } else { +// intp = createRepl(info.getPath(), +// info.getClassName(), +// properties); +// } synchronized (interpreterGroup) { List interpreters = interpreterGroup.get(key); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java index f9e43abfcae..4d8246e182d 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java @@ -21,7 +21,7 @@ * */ public class InterpreterOption { - boolean remote; +// boolean remote; boolean perNoteSession; boolean perNoteProcess; @@ -58,21 +58,21 @@ public void setHost(String host) { } - public InterpreterOption() { - remote = false; - } +// public InterpreterOption() { +// remote = false; +// } - public InterpreterOption(boolean remote) { - this.remote = remote; - } +// public InterpreterOption(boolean remote) { +// this.remote = remote; +// } - public boolean isRemote() { - return remote; - } +// public boolean isRemote() { +// return remote; +// } - public void setRemote(boolean remote) { - this.remote = remote; - } +// public void setRemote(boolean remote) { +// this.remote = remote; +// } public boolean isPerNoteSession() { return perNoteSession; diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java index 3d9ee6ff5cb..9c6ec072697 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java @@ -57,7 +57,7 @@ public void setUp() throws Exception { System.setProperty(ConfVars.ZEPPELIN_INTERPRETERS.getVarName(), "org.apache.zeppelin.interpreter.mock.MockInterpreter1,org.apache.zeppelin.interpreter.mock.MockInterpreter2"); conf = new ZeppelinConfiguration(); depResolver = new DependencyResolver(tmpDir.getAbsolutePath() + "/local-repo"); - factory = new InterpreterFactory(conf, new InterpreterOption(false), null, null, depResolver); + factory = new InterpreterFactory(conf, new InterpreterOption(), null, null, depResolver); context = new InterpreterContext("note", "id", "title", "text", null, null, null, null, null, null, null); } @@ -115,7 +115,7 @@ public void testExceptions() throws InterpreterException, IOException, Repositor assertEquals("Test null option" , e.getMessage(),new NullArgumentException("option").getMessage()); } try { - factory.add("a mock", "mock2", new LinkedList(), new InterpreterOption(false), null); + factory.add("a mock", "mock2", new LinkedList(), new InterpreterOption(), null); } catch (NullArgumentException e){ assertEquals("Test null properties" , e.getMessage(),new NullArgumentException("properties").getMessage()); } @@ -130,7 +130,7 @@ public void testSaveLoad() throws IOException, RepositoryException { // check if file saved assertTrue(new File(conf.getInterpreterSettingPath()).exists()); - factory.add("newsetting", "mock1", new LinkedList(), new InterpreterOption(false), new Properties()); + factory.add("newsetting", "mock1", new LinkedList(), new InterpreterOption(), new Properties()); assertEquals(3, factory.get().size()); InterpreterFactory factory2 = new InterpreterFactory(conf, null, null, null, depResolver); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java index f6abc9dc704..777eebf8b28 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java @@ -61,7 +61,7 @@ public void setUp() throws Exception { MockInterpreter2.register("mock2", "group2", "org.apache.zeppelin.interpreter.mock.MockInterpreter2"); depResolver = new DependencyResolver(tmpDir.getAbsolutePath() + "/local-repo"); - factory = new InterpreterFactory(conf, new InterpreterOption(false), null, null, depResolver); + factory = new InterpreterFactory(conf, new InterpreterOption(), null, null, depResolver); } @After diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java index 53749d1ddea..20409f5377e 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java @@ -90,7 +90,7 @@ public void setUp() throws Exception { MockInterpreter2.register("mock2", "org.apache.zeppelin.interpreter.mock.MockInterpreter2"); depResolver = new DependencyResolver(tmpDir.getAbsolutePath() + "/local-repo"); - factory = new InterpreterFactory(conf, new InterpreterOption(false), null, null, depResolver); + factory = new InterpreterFactory(conf, new InterpreterOption(), null, null, depResolver); SearchService search = mock(SearchService.class); notebookRepo = new VFSNotebookRepo(conf); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java index 1699d681bdf..468073a3009 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java @@ -94,7 +94,7 @@ public void setUp() throws Exception { MockInterpreter2.register("mock2", "org.apache.zeppelin.interpreter.mock.MockInterpreter2"); depResolver = new DependencyResolver(mainZepDir.getAbsolutePath() + "/local-repo"); - factory = new InterpreterFactory(conf, new InterpreterOption(false), null, null, depResolver); + factory = new InterpreterFactory(conf, new InterpreterOption(), null, null, depResolver); search = mock(SearchService.class); notebookRepoSync = new NotebookRepoSync(conf); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java index e5915a97299..fea8d47db76 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java @@ -76,7 +76,7 @@ public void setUp() throws Exception { this.schedulerFactory = new SchedulerFactory(); depResolver = new DependencyResolver(mainZepDir.getAbsolutePath() + "/local-repo"); - factory = new InterpreterFactory(conf, new InterpreterOption(false), null, null, depResolver); + factory = new InterpreterFactory(conf, new InterpreterOption(), null, null, depResolver); SearchService search = mock(SearchService.class); notebookRepo = new VFSNotebookRepo(conf); From 004dc133573374babe3df57c0698328c11094eb5 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Sun, 12 Jun 2016 00:27:49 +0900 Subject: [PATCH 2/3] Removed commented codes Reverted remote option and marked deprecated --- .../interpreter/InterpreterFactory.java | 15 ------------ .../interpreter/InterpreterOption.java | 24 +++++++------------ 2 files changed, 9 insertions(+), 30 deletions(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java index a8fd4b5fe30..2be2a300759 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java @@ -312,7 +312,6 @@ private void loadFromFile() throws IOException { // While we decided to turn this feature on always (without providing // enable/disable option on GUI). // previously created setting should turn this feature on here. -// setting.getOption().setRemote(true); InterpreterSetting intpSetting = new InterpreterSetting( setting.id(), @@ -499,19 +498,11 @@ public InterpreterGroup createInterpreterGroup(String id, InterpreterOption opti AngularObjectRegistry angularObjectRegistry; InterpreterGroup interpreterGroup = new InterpreterGroup(id); -// if (option.isRemote()) { angularObjectRegistry = new RemoteAngularObjectRegistry( id, angularObjectRegistryListener, interpreterGroup ); -// } else { -// angularObjectRegistry = new AngularObjectRegistry( -// id, -// angularObjectRegistryListener); -// -// // TODO(moon) : create distributed resource pool for local interpreters and set -// } interpreterGroup.setAngularObjectRegistry(angularObjectRegistry); return interpreterGroup; @@ -581,17 +572,11 @@ public void createInterpretersForNote( && info.getGroup().equals(groupName)) { Interpreter intp; -// if (option.isRemote()) { intp = createRemoteRepl(info.getPath(), key, info.getClassName(), properties, interpreterSetting.id()); -// } else { -// intp = createRepl(info.getPath(), -// info.getClassName(), -// properties); -// } synchronized (interpreterGroup) { List interpreters = interpreterGroup.get(key); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java index 4d8246e182d..85b0c82e818 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java @@ -21,7 +21,7 @@ * */ public class InterpreterOption { -// boolean remote; + boolean remote = false; boolean perNoteSession; boolean perNoteProcess; @@ -58,21 +58,15 @@ public void setHost(String host) { } -// public InterpreterOption() { -// remote = false; -// } - -// public InterpreterOption(boolean remote) { -// this.remote = remote; -// } - -// public boolean isRemote() { -// return remote; -// } + @Deprecated + public boolean isRemote() { + return remote; + } -// public void setRemote(boolean remote) { -// this.remote = remote; -// } + @Deprecated + public void setRemote(boolean remote) { + this.remote = remote; + } public boolean isPerNoteSession() { return perNoteSession; From 62a65fd323906f3f0ad20cd1de18eac17b5be935 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Wed, 15 Jun 2016 01:32:05 +0900 Subject: [PATCH 3/3] Fixed wrong default value of InterpreterOption --- .../java/org/apache/zeppelin/interpreter/InterpreterOption.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java index 85b0c82e818..7bf9a4b19e1 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java @@ -21,7 +21,7 @@ * */ public class InterpreterOption { - boolean remote = false; + boolean remote = true; boolean perNoteSession; boolean perNoteProcess;