You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2018/03/14 12:52:57 UTC
zeppelin git commit: ZEPPELIN-3315. Merge beforeStatusChange and
afterStatusChange to onStatusChange
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 <zj...@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 <zj...@apache.org>
Authored: Mon Mar 12 10:12:51 2018 +0800
Committer: Jeff Zhang <zj...@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) {
}
};
}