Repository: zeppelin Updated Branches: refs/heads/master 1effc6350 -> 4ee09717c
ZEPPELIN-3315. Merge beforeStatusChange and afterStatusChange to onStatusChange ### What is this PR for? The signature of `beforeStatusChange` & `afterStatusChange` include both the status of before and after, so it is not necessary to create both `beforeStatusChange` & `afterStatusChange`. Only one method `onStatusChange` is sufficient. ### What type of PR is it? [Refactoring] ### Todos * [ ] - Task ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-3315 ### How should this be tested? * CI pass ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Jeff Zhang <zjf...@apache.org> Closes #2858 from zjffdu/ZEPPELIN-3315 and squashes the following commits: f0fe594 [Jeff Zhang] ZEPPELIN-3315. Merge beforeStatusChange and afterStatusChange to onStatusChange Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/4ee09717 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/4ee09717 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/4ee09717 Branch: refs/heads/master Commit: 4ee09717cfb2d09b55f20fbd8245b6c2eca37fda Parents: 1effc63 Author: Jeff Zhang <zjf...@apache.org> Authored: Mon Mar 12 10:12:51 2018 +0800 Committer: Jeff Zhang <zjf...@apache.org> Committed: Wed Mar 14 20:52:46 2018 +0800 ---------------------------------------------------------------------- .../interpreter/remote/RemoteInterpreterServer.java | 6 +----- .../java/org/apache/zeppelin/scheduler/Job.java | 7 ++----- .../org/apache/zeppelin/scheduler/JobListener.java | 6 ++---- .../org/apache/zeppelin/socket/NotebookServer.java | 6 +----- .../java/org/apache/zeppelin/notebook/Note.java | 16 +++------------- .../apache/zeppelin/scheduler/RemoteScheduler.java | 10 ++-------- .../helium/HeliumApplicationFactoryTest.java | 7 +------ .../org/apache/zeppelin/notebook/NotebookTest.java | 6 +----- .../notebook/repo/NotebookRepoSyncTest.java | 5 +---- 9 files changed, 14 insertions(+), 55 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java index d50d0ed..766ed16 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java @@ -513,11 +513,7 @@ public class RemoteInterpreterServer extends Thread } @Override - public void beforeStatusChange(Job job, Status before, Status after) { - } - - @Override - public void afterStatusChange(Job job, Status before, Status after) { + public void onStatusChange(Job job, Status before, Status after) { synchronized (this) { notifyAll(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java index 8e25f7b..579b604 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java @@ -152,12 +152,9 @@ public abstract class Job { } Status before = this.status; Status after = status; - if (listener != null) { - listener.beforeStatusChange(this, before, after); - } this.status = status; - if (listener != null) { - listener.afterStatusChange(this, before, after); + if (listener != null && before != after) { + listener.onStatusChange(this, before, after); } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/JobListener.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/JobListener.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/JobListener.java index 3042941..00f8d81 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/JobListener.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/JobListener.java @@ -18,12 +18,10 @@ package org.apache.zeppelin.scheduler; /** - * TODO(moon) : add description. + * Listener for job execution. */ public interface JobListener { void onProgressUpdate(Job job, int progress); - void beforeStatusChange(Job job, Job.Status before, Job.Status after); - - void afterStatusChange(Job job, Job.Status before, Job.Status after); + void onStatusChange(Job job, Job.Status before, Job.Status after); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java index 0888874..4a17f5d 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java @@ -2279,11 +2279,7 @@ public class NotebookServer extends WebSocketServlet } @Override - public void beforeStatusChange(Job job, Status before, Status after) { - } - - @Override - public void afterStatusChange(Job job, Status before, Status after) { + public void onStatusChange(Job job, Status before, Status after) { if (after == Status.ERROR) { if (job.getException() != null) { LOG.error("Error", job.getException()); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java index fc70c70..f2d7763 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java @@ -912,23 +912,13 @@ public class Note implements ParagraphJobListener, JsonSerializable { public void setInfo(Map<String, Object> info) { this.info = info; } - - @Override - public void beforeStatusChange(Job job, Status before, Status after) { - if (jobListenerFactory != null) { - ParagraphJobListener listener = jobListenerFactory.getParagraphJobListener(this); - if (listener != null) { - listener.beforeStatusChange(job, before, after); - } - } - } - + @Override - public void afterStatusChange(Job job, Status before, Status after) { + public void onStatusChange(Job job, Status before, Status after) { if (jobListenerFactory != null) { ParagraphJobListener listener = jobListenerFactory.getParagraphJobListener(this); if (listener != null) { - listener.afterStatusChange(job, before, after); + listener.onStatusChange(job, before, after); } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java index 982b84a..d6d1df7 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java @@ -253,12 +253,10 @@ public class RemoteScheduler implements Scheduler { if (status == Status.UNKNOWN) { // not found this job in the remote schedulers. // maybe not submitted, maybe already finished - //Status status = getLastStatus(); - listener.afterStatusChange(job, null, null); return job.getStatus(); } lastStatus = status; - listener.afterStatusChange(job, null, status); + listener.onStatusChange(job, null, status); return status; } } @@ -350,11 +348,7 @@ public class RemoteScheduler implements Scheduler { } @Override - public void beforeStatusChange(Job job, Status before, Status after) { - } - - @Override - public void afterStatusChange(Job job, Status before, Status after) { + public void onStatusChange(Job job, Status before, Status after) { // Update remoteStatus if (jobExecuted == false) { if (after == Status.FINISHED || after == Status.ABORT http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java index 03de533..af91819 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java @@ -322,12 +322,7 @@ public class HeliumApplicationFactoryTest extends AbstractInterpreterTest implem } @Override - public void beforeStatusChange(Job job, Job.Status before, Job.Status after) { - - } - - @Override - public void afterStatusChange(Job job, Job.Status before, Job.Status after) { + public void onStatusChange(Job job, Job.Status before, Job.Status after) { } }; http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java ---------------------------------------------------------------------- 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 cfb2e16..490ac53 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 @@ -1460,11 +1460,7 @@ public class NotebookTest extends AbstractInterpreterTest implements JobListener } @Override - public void beforeStatusChange(Job job, Status before, Status after) { - } - - @Override - public void afterStatusChange(Job job, Status before, Status after) { + public void onStatusChange(Job job, Status before, Status after) { if (afterStatusChangedListener != null) { afterStatusChangedListener.onStatusChanged(job, before, after); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4ee09717/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java ---------------------------------------------------------------------- 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 8904239..5571230 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 @@ -428,11 +428,8 @@ public class NotebookRepoSyncTest implements JobListenerFactory { } @Override - public void beforeStatusChange(Job job, Status before, Status after) { - } + public void onStatusChange(Job job, Status before, Status after) { - @Override - public void afterStatusChange(Job job, Status before, Status after) { } }; }
