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) {
       }
     };
   }

Reply via email to