Repository: zeppelin Updated Branches: refs/heads/master 9be827a60 -> 29b743609
[ZEPPELIN-1352] Refactor Notebook repo versioning to return Revision Empty instead of NULL ### What is this PR for? In Zeppelin notebookRepo, versioning (checkpoint, get, revisionHistory) use to return NULL. It will be better to return Empty (Revision or list). ### What type of PR is it? [Improvement] ### What is the Jira issue? * [ZEPPELIN-1352](https://issues.apache.org/jira/browse/ZEPPELIN-1352) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Anthony Corbacho <corbacho.anth...@gmail.com> Closes #1697 from anthonycorbacho/ZEPPELIN-1352 and squashes the following commits: 974c976 [Anthony Corbacho] Fix test in GitNotebookRepo 058b3e7 [Anthony Corbacho] Apply return empty revision instead of null in zeppelinhub notebook repo 5127961 [Anthony Corbacho] Change logic to return Empty (Revision or Collection) instead of null 989a995 [Anthony Corbacho] Add to Revision class a static final empty object that will be used to return empty revision instad of null Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/29b74360 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/29b74360 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/29b74360 Branch: refs/heads/master Commit: 29b7436091c52975e9b3bbd28868f379b55eaa63 Parents: 9be827a Author: Anthony Corbacho <corbacho.anth...@gmail.com> Authored: Mon Nov 28 11:31:10 2016 +0900 Committer: Anthony Corbacho <corbacho.anth...@gmail.com> Committed: Thu Dec 1 23:24:45 2016 +0900 ---------------------------------------------------------------------- .../apache/zeppelin/notebook/repo/AzureNotebookRepo.java | 10 +++++----- .../apache/zeppelin/notebook/repo/GitNotebookRepo.java | 2 +- .../org/apache/zeppelin/notebook/repo/NotebookRepo.java | 3 +++ .../apache/zeppelin/notebook/repo/S3NotebookRepo.java | 11 ++++++----- .../apache/zeppelin/notebook/repo/VFSNotebookRepo.java | 11 ++++++----- .../notebook/repo/zeppelinhub/ZeppelinHubRepo.java | 2 +- .../zeppelin/notebook/repo/GitNotebookRepoTest.java | 3 ++- 7 files changed, 24 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/29b74360/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java index 05f2fdc..b0b9782 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java @@ -224,20 +224,20 @@ public class AzureNotebookRepo implements NotebookRepo { public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationInfo subject) throws IOException { // no-op - LOG.info("Checkpoint feature isn't supported in {}", this.getClass().toString()); - return null; + LOG.warn("Checkpoint feature isn't supported in {}", this.getClass().toString()); + return Revision.EMPTY; } @Override public Note get(String noteId, String revId, AuthenticationInfo subject) throws IOException { - // Auto-generated method stub + LOG.warn("Get note revision feature isn't supported in {}", this.getClass().toString()); return null; } @Override public List<Revision> revisionHistory(String noteId, AuthenticationInfo subject) { - // Auto-generated method stub - return null; + LOG.warn("Get Note revisions feature isn't supported in {}", this.getClass().toString()); + return Collections.emptyList(); } @Override http://git-wip-us.apache.org/repos/asf/zeppelin/blob/29b74360/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java index b0678dd..407795e 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java @@ -82,7 +82,7 @@ public class GitNotebookRepo extends VFSNotebookRepo { */ @Override public Revision checkpoint(String pattern, String commitMessage, AuthenticationInfo subject) { - Revision revision = null; + Revision revision = Revision.EMPTY; try { List<DiffEntry> gitDiff = git.diff().call(); if (!gitDiff.isEmpty()) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/29b74360/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java index 8016217..5ac9702 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.annotation.ZeppelinApi; import org.apache.zeppelin.notebook.Note; import org.apache.zeppelin.notebook.NoteInfo; @@ -121,6 +122,8 @@ public interface NotebookRepo { * Represents the 'Revision' a point in life of the notebook */ static class Revision { + public static final Revision EMPTY = new Revision(StringUtils.EMPTY, StringUtils.EMPTY, 0); + public Revision(String revId, String message, int time) { this.id = revId; this.message = message; http://git-wip-us.apache.org/repos/asf/zeppelin/blob/29b74360/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java index 889fd03..045bc9d 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java @@ -257,20 +257,21 @@ public class S3NotebookRepo implements NotebookRepo { @Override public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationInfo subject) throws IOException { - // Auto-generated method stub - return null; + // no-op + LOG.warn("Checkpoint feature isn't supported in {}", this.getClass().toString()); + return Revision.EMPTY; } @Override public Note get(String noteId, String revId, AuthenticationInfo subject) throws IOException { - // Auto-generated method stub + LOG.warn("Get note revision feature isn't supported in {}", this.getClass().toString()); return null; } @Override public List<Revision> revisionHistory(String noteId, AuthenticationInfo subject) { - // Auto-generated method stub - return null; + LOG.warn("Get Note revisions feature isn't supported in {}", this.getClass().toString()); + return Collections.emptyList(); } @Override http://git-wip-us.apache.org/repos/asf/zeppelin/blob/29b74360/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java index 3820d71..b8510d5 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java @@ -276,20 +276,21 @@ public class VFSNotebookRepo implements NotebookRepo { @Override public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationInfo subject) throws IOException { - // Auto-generated method stub - return null; + // no-op + LOG.warn("Checkpoint feature isn't supported in {}", this.getClass().toString()); + return Revision.EMPTY; } @Override public Note get(String noteId, String revId, AuthenticationInfo subject) throws IOException { - // Auto-generated method stub + LOG.warn("Get note revision feature isn't supported in {}", this.getClass().toString()); return null; } @Override public List<Revision> revisionHistory(String noteId, AuthenticationInfo subject) { - // Auto-generated method stub - return null; + LOG.warn("Get Note revisions feature isn't supported in {}", this.getClass().toString()); + return Collections.emptyList(); } @Override http://git-wip-us.apache.org/repos/asf/zeppelin/blob/29b74360/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java index 9c9f7f7..8141b1c 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java @@ -269,7 +269,7 @@ public class ZeppelinHubRepo implements NotebookRepo { public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationInfo subject) throws IOException { if (StringUtils.isBlank(noteId) || !isSubjectValid(subject)) { - return null; + return Revision.EMPTY; } String endpoint = Joiner.on("/").join(noteId, "checkpoint"); String content = GSON.toJson(ImmutableMap.of("message", checkpointMsg)); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/29b74360/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java index 2e96615..e46b9d0 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java @@ -150,7 +150,8 @@ public class GitNotebookRepoTest { assertThat(notebookRepo.checkpoint(TEST_NOTE_ID, "second commit, note1", null)).isNotNull(); assertThat(notebookRepo.revisionHistory(TEST_NOTE_ID, null).size()).isEqualTo(2); assertThat(notebookRepo.revisionHistory(TEST_NOTE_ID2, null).size()).isEqualTo(1); - assertThat(notebookRepo.checkpoint(TEST_NOTE_ID2, "first commit, note2", null)).isNull(); + assertThat(notebookRepo.checkpoint(TEST_NOTE_ID2, "first commit, note2", null)) + .isEqualTo(Revision.EMPTY); assertThat(notebookRepo.revisionHistory(TEST_NOTE_ID2, null).size()).isEqualTo(1); //modify, save and checkpoint second note
