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 2019/03/12 07:19:23 UTC
[zeppelin] branch master updated: [ZEPPELIN-4051]. Commit note is
broken
This is an automated email from the ASF dual-hosted git repository.
zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 4f5297d [ZEPPELIN-4051]. Commit note is broken
4f5297d is described below
commit 4f5297d0d33abfa588f1ee7aa860a4bec6bffd52
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Mon Mar 11 14:43:50 2019 +0800
[ZEPPELIN-4051]. Commit note is broken
### What is this PR for?
Commit note is broken due to wrong argument is passed (due to ZEPPELIN-2619). The correct parameter passed to NotebookRepo should be note path instead of note name.
### What type of PR is it?
[Bug Fix]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://jira.apache.org/jira/browse/ZEPPELIN-4051
### 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 #3328 from zjffdu/ZEPPELIN-4051 and squashes the following commits:
9d35ef105 [Jeff Zhang] [ZEPPELIN-4051]. Commit note is broken
---
.../zeppelin/notebook/repo/GitNotebookRepo.java | 4 +--
.../apache/zeppelin/service/NotebookService.java | 2 +-
.../apache/zeppelin/socket/NotebookServerTest.java | 30 ++++++++++++++++++++++
.../src/test/resources/zeppelin-site.xml | 2 +-
.../src/app/notebook/notebook.controller.js | 2 +-
.../org/apache/zeppelin/notebook/Notebook.java | 12 ++++-----
.../zeppelin/notebook/repo/NotebookRepoSync.java | 16 ++++++------
.../repo/NotebookRepoWithVersionControl.java | 16 ++++++------
.../org/apache/zeppelin/notebook/NotebookTest.java | 8 +++---
9 files changed, 61 insertions(+), 31 deletions(-)
diff --git a/zeppelin-plugins/notebookrepo/git/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java b/zeppelin-plugins/notebookrepo/git/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java
index 2da91fc..322d692 100644
--- a/zeppelin-plugins/notebookrepo/git/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java
+++ b/zeppelin-plugins/notebookrepo/git/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java
@@ -214,10 +214,10 @@ public class GitNotebookRepo extends VFSNotebookRepo implements NotebookRepoWith
}
@Override
- public Note setNoteRevision(String noteId, String noteName, String revId,
+ public Note setNoteRevision(String noteId, String notePath, String revId,
AuthenticationInfo subject)
throws IOException {
- Note revisionNote = get(noteId, noteName, revId, subject);
+ Note revisionNote = get(noteId, notePath, revId, subject);
if (revisionNote != null) {
save(revisionNote, subject);
}
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
index 62973ff..ea4df53 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
@@ -700,7 +700,7 @@ public class NotebookService {
}
NotebookRepoWithVersionControl.Revision revision =
- notebook.checkpointNote(noteId, note.getName(), commitMessage, context.getAutheInfo());
+ notebook.checkpointNote(noteId, note.getPath(), commitMessage, context.getAutheInfo());
callback.onSuccess(revision, context);
return revision;
}
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
index 0760e6a..1e9071d 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
@@ -55,6 +55,8 @@ import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.Notebook;
import org.apache.zeppelin.notebook.NotebookAuthorization;
import org.apache.zeppelin.notebook.Paragraph;
+import org.apache.zeppelin.notebook.repo.NotebookRepoWithVersionControl;
+import org.apache.zeppelin.notebook.repo.zeppelinhub.security.Authentication;
import org.apache.zeppelin.notebook.socket.Message;
import org.apache.zeppelin.notebook.socket.Message.OP;
import org.apache.zeppelin.rest.AbstractTestRestApi;
@@ -695,6 +697,34 @@ public class NotebookServerTest extends AbstractTestRestApi {
assertNotNull(user1Id + " can get " + user2Id + "'s shared note", paragraphList2);
}
+ @Test
+ public void testNoteRevision() throws IOException {
+ Note note = notebook.createNote("note1", anonymous);
+ assertEquals(0, note.getParagraphCount());
+ NotebookRepoWithVersionControl.Revision firstRevision = notebook.checkpointNote(note.getId(), note.getPath(), "first commit", AuthenticationInfo.ANONYMOUS);
+ List<NotebookRepoWithVersionControl.Revision> revisionList = notebook.listRevisionHistory(note.getId(), note.getPath(), AuthenticationInfo.ANONYMOUS);
+ assertEquals(1, revisionList.size());
+ assertEquals(firstRevision.id, revisionList.get(0).id);
+ assertEquals("first commit", revisionList.get(0).message);
+
+ // add one new paragraph and commit it
+ note.addNewParagraph(AuthenticationInfo.ANONYMOUS);
+ notebook.saveNote(note, AuthenticationInfo.ANONYMOUS);
+ assertEquals(1, note.getParagraphCount());
+ NotebookRepoWithVersionControl.Revision secondRevision = notebook.checkpointNote(note.getId(), note.getPath(), "second commit", AuthenticationInfo.ANONYMOUS);
+
+ revisionList = notebook.listRevisionHistory(note.getId(), note.getPath(), AuthenticationInfo.ANONYMOUS);
+ assertEquals(2, revisionList.size());
+ assertEquals(secondRevision.id, revisionList.get(0).id);
+ assertEquals("second commit", revisionList.get(0).message);
+ assertEquals(firstRevision.id, revisionList.get(1).id);
+ assertEquals("first commit", revisionList.get(1).message);
+
+ // checkout the first commit
+ note = notebook.getNoteByRevision(note.getId(), note.getPath(), firstRevision.id, AuthenticationInfo.ANONYMOUS);
+ assertEquals(0, note.getParagraphCount());
+ }
+
private NotebookSocket createWebSocket() {
NotebookSocket sock = mock(NotebookSocket.class);
when(sock.getRequest()).thenReturn(mockRequest);
diff --git a/zeppelin-server/src/test/resources/zeppelin-site.xml b/zeppelin-server/src/test/resources/zeppelin-site.xml
index 3215c5b..e46fce7 100644
--- a/zeppelin-server/src/test/resources/zeppelin-site.xml
+++ b/zeppelin-server/src/test/resources/zeppelin-site.xml
@@ -73,7 +73,7 @@
<property>
<name>zeppelin.notebook.storage</name>
- <value>org.apache.zeppelin.notebook.repo.VFSNotebookRepo</value>
+ <value>org.apache.zeppelin.notebook.repo.GitNotebookRepo</value>
<description>notebook persistence layer implementation</description>
</property>
diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js
index 426667d..085c94e 100644
--- a/zeppelin-web/src/app/notebook/notebook.controller.js
+++ b/zeppelin-web/src/app/notebook/notebook.controller.js
@@ -289,7 +289,7 @@ function NotebookCtrl($scope, $route, $routeParams, $location, $rootScope,
message: 'Commit note to current repository?',
callback: function(result) {
if (result) {
- websocketMsgSrv.checkpointNote($routeParams.noteId, $routeParams.name, commitMessage);
+ websocketMsgSrv.checkpointNote($routeParams.noteId, commitMessage);
}
},
});
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
index ab32059..e7f5ff0 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
@@ -325,32 +325,32 @@ public class Notebook {
}
}
- public Revision checkpointNote(String noteId, String noteName, String checkpointMessage,
+ public Revision checkpointNote(String noteId, String notePath, String checkpointMessage,
AuthenticationInfo subject) throws IOException {
if (((NotebookRepoSync) notebookRepo).isRevisionSupportedInDefaultRepo()) {
return ((NotebookRepoWithVersionControl) notebookRepo)
- .checkpoint(noteId, noteName, checkpointMessage, subject);
+ .checkpoint(noteId, notePath, checkpointMessage, subject);
} else {
return null;
}
}
public List<Revision> listRevisionHistory(String noteId,
- String noteName,
+ String notePath,
AuthenticationInfo subject) throws IOException {
if (((NotebookRepoSync) notebookRepo).isRevisionSupportedInDefaultRepo()) {
return ((NotebookRepoWithVersionControl) notebookRepo)
- .revisionHistory(noteId, noteName, subject);
+ .revisionHistory(noteId, notePath, subject);
} else {
return null;
}
}
- public Note setNoteRevision(String noteId, String noteName, String revisionId, AuthenticationInfo subject)
+ public Note setNoteRevision(String noteId, String notePath, String revisionId, AuthenticationInfo subject)
throws IOException {
if (((NotebookRepoSync) notebookRepo).isRevisionSupportedInDefaultRepo()) {
return ((NotebookRepoWithVersionControl) notebookRepo)
- .setNoteRevision(noteId, noteName, revisionId, subject);
+ .setNoteRevision(noteId, notePath, revisionId, subject);
} else {
return null;
}
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
index be20cfb..b6efdc3 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
@@ -483,7 +483,7 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl {
//checkpoint to all available storages
@Override
- public Revision checkpoint(String noteId, String noteName, String checkpointMsg, AuthenticationInfo subject)
+ public Revision checkpoint(String noteId, String notePath, String checkpointMsg, AuthenticationInfo subject)
throws IOException {
int repoCount = getRepoCount();
int repoBound = Math.min(repoCount, getMaxRepoNum());
@@ -496,7 +496,7 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl {
if (isRevisionSupportedInRepo(i)) {
allRepoCheckpoints
.add(((NotebookRepoWithVersionControl) getRepo(i))
- .checkpoint(noteId, noteName, checkpointMsg, subject));
+ .checkpoint(noteId, notePath, checkpointMsg, subject));
}
} catch (IOException e) {
LOGGER.warn("Couldn't checkpoint in {} storage with index {} for note {}",
@@ -521,11 +521,11 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl {
}
@Override
- public Note get(String noteId, String noteName, String revId, AuthenticationInfo subject) {
+ public Note get(String noteId, String notePath, String revId, AuthenticationInfo subject) {
Note revisionNote = null;
try {
if (isRevisionSupportedInDefaultRepo()) {
- revisionNote = ((NotebookRepoWithVersionControl) getRepo(0)).get(noteId, noteName,
+ revisionNote = ((NotebookRepoWithVersionControl) getRepo(0)).get(noteId, notePath,
revId, subject);
}
} catch (IOException e) {
@@ -535,13 +535,13 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl {
}
@Override
- public List<Revision> revisionHistory(String noteId, String noteName,
+ public List<Revision> revisionHistory(String noteId, String notePath,
AuthenticationInfo subject) {
List<Revision> revisions = Collections.emptyList();
try {
if (isRevisionSupportedInDefaultRepo()) {
revisions = ((NotebookRepoWithVersionControl) getRepo(0))
- .revisionHistory(noteId, noteName, subject);
+ .revisionHistory(noteId, notePath, subject);
}
} catch (IOException e) {
LOGGER.error("Failed to list revision history", e);
@@ -570,7 +570,7 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl {
}
@Override
- public Note setNoteRevision(String noteId, String noteName, String revId, AuthenticationInfo subject)
+ public Note setNoteRevision(String noteId, String notePath, String revId, AuthenticationInfo subject)
throws IOException {
int repoCount = getRepoCount();
int repoBound = Math.min(repoCount, getMaxRepoNum());
@@ -579,7 +579,7 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl {
try {
if (isRevisionSupportedInRepo(i)) {
currentNote = ((NotebookRepoWithVersionControl) getRepo(i))
- .setNoteRevision(noteId, noteName, revId, subject);
+ .setNoteRevision(noteId, notePath, revId, subject);
}
} catch (IOException e) {
// already logged
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoWithVersionControl.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoWithVersionControl.java
index ba5f4cf..a734c1a 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoWithVersionControl.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoWithVersionControl.java
@@ -35,13 +35,13 @@ public interface NotebookRepoWithVersionControl extends NotebookRepo {
/**
* chekpoint (set revision) for notebook.
* @param noteId Id of the note
- * @param noteName name of the note
+ * @param notePath path of the note
* @param checkpointMsg message description of the checkpoint
* @return Rev
* @throws IOException
*/
@ZeppelinApi Revision checkpoint(String noteId,
- String noteName,
+ String notePath,
String checkpointMsg,
AuthenticationInfo subject) throws IOException;
@@ -49,36 +49,36 @@ public interface NotebookRepoWithVersionControl extends NotebookRepo {
* Get particular revision of the Notebook.
*
* @param noteId Id of the note
- * @param noteName name of the note
+ * @param notePath path of the note
* @param revId revision of the Notebook
* @return a Notebook
* @throws IOException
*/
- @ZeppelinApi Note get(String noteId, String noteName, String revId, AuthenticationInfo subject)
+ @ZeppelinApi Note get(String noteId, String notePath, String revId, AuthenticationInfo subject)
throws IOException;
/**
* List of revisions of the given Notebook.
*
* @param noteId id of the note
- * @param noteName name of the note
+ * @param notePath path of the note
* @param subject
* @return list of revisions
*/
@ZeppelinApi List<Revision> revisionHistory(String noteId,
- String noteName,
+ String notePath,
AuthenticationInfo subject) throws IOException;
/**
* Set note to particular revision.
*
* @param noteId Id of the Notebook
- * @param noteName name of the note
+ * @param notePath path of the note
* @param revId revision of the Notebook
* @return a Notebook
* @throws IOException
*/
- @ZeppelinApi Note setNoteRevision(String noteId, String noteName, String revId,
+ @ZeppelinApi Note setNoteRevision(String noteId, String notePath, String revId,
AuthenticationInfo subject) throws IOException;
/**
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 8b1527d..7feb1e8 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
@@ -187,23 +187,23 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
NotebookRepoWithVersionControl {
@Override
- public Revision checkpoint(String noteId, String noteName, String checkpointMsg, AuthenticationInfo subject)
+ public Revision checkpoint(String noteId, String notePath, String checkpointMsg, AuthenticationInfo subject)
throws IOException {
return null;
}
@Override
- public Note get(String noteId, String noteName, String revId, AuthenticationInfo subject) throws IOException {
+ public Note get(String noteId, String notePath, String revId, AuthenticationInfo subject) throws IOException {
return null;
}
@Override
- public List<Revision> revisionHistory(String noteId, String noteName, AuthenticationInfo subject) {
+ public List<Revision> revisionHistory(String noteId, String notePath, AuthenticationInfo subject) {
return null;
}
@Override
- public Note setNoteRevision(String noteId, String noteName, String revId, AuthenticationInfo subject) throws
+ public Note setNoteRevision(String noteId, String notePath, String revId, AuthenticationInfo subject) throws
IOException {
return null;
}