Skip to content

[ZEPPELIN-1257] storage - fix get note revision api #1254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
khalidhuseynov wants to merge 7 commits into from
Closed

[ZEPPELIN-1257] storage - fix get note revision api #1254

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d90c104
change api Revision -> String (revision id)
khalidhuseynov Jul 31, 2016
b00b9e7
change Revision -> String in NotebookServer
khalidhuseynov Jul 31, 2016
da7a612
consistency check: NotebookRepo.Revision -> Revision
khalidhuseynov Jul 31, 2016
6ead891
fix tests
khalidhuseynov Aug 1, 2016
12ed9f5
revision -> revisionId on front
khalidhuseynov Aug 12, 2016
b0f156a
getNoteRevision -> getNoteByRevision
khalidhuseynov Aug 12, 2016
a8d005b
notebook: getNoteRevision -> getNoteByRevision
khalidhuseynov Aug 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcessListener;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.notebook.*;
import org.apache.zeppelin.notebook.repo.NotebookRepo;
import org.apache.zeppelin.notebook.repo.NotebookRepo.Revision;
import org.apache.zeppelin.notebook.socket.Message;
import org.apache.zeppelin.notebook.socket.Message.OP;
Expand Down Expand Up @@ -237,7 +236,7 @@ public void onMessage(NotebookSocket conn, String msg) {
listRevisionHistory(conn, notebook, messagereceived);
break;
case NOTE_REVISION:
getNoteRevision(conn, notebook, messagereceived);
getNoteByRevision(conn, notebook, messagereceived);
break;
case LIST_NOTEBOOK_JOBS:
unicastNotebookJobInfo(conn, messagereceived);
Expand Down Expand Up @@ -1182,7 +1181,7 @@ private void checkpointNotebook(NotebookSocket conn, Notebook notebook,
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
Revision revision = notebook.checkpointNote(noteId, commitMessage, subject);
if (revision != null) {
List<NotebookRepo.Revision> revisions = notebook.listRevisionHistory(noteId, subject);
List<Revision> revisions = notebook.listRevisionHistory(noteId, subject);
conn.send(serializeMessage(new Message(OP.LIST_REVISION_HISTORY)
.put("revisionList", revisions)));
}
Expand All @@ -1192,21 +1191,21 @@ private void listRevisionHistory(NotebookSocket conn, Notebook notebook,
Message fromMessage) throws IOException {
String noteId = (String) fromMessage.get("noteId");
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
List<NotebookRepo.Revision> revisions = notebook.listRevisionHistory(noteId, subject);
List<Revision> revisions = notebook.listRevisionHistory(noteId, subject);

conn.send(serializeMessage(new Message(OP.LIST_REVISION_HISTORY)
.put("revisionList", revisions)));
}

private void getNoteRevision(NotebookSocket conn, Notebook notebook, Message fromMessage)
private void getNoteByRevision(NotebookSocket conn, Notebook notebook, Message fromMessage)
throws IOException {
String noteId = (String) fromMessage.get("noteId");
Revision revision = (Revision) fromMessage.get("revision");
String revisionId = (String) fromMessage.get("revisionId");
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
Note revisionNote = notebook.getNoteRevision(noteId, revision, subject);
Note revisionNote = notebook.getNoteRevision(noteId, revisionId, subject);
conn.send(serializeMessage(new Message(OP.NOTE_REVISION)
.put("noteId", noteId)
.put("revisionId", revision)
.put("revisionId", revisionId)
.put("data", revisionNote)));
}

Expand Down
4 changes: 2 additions & 2 deletions zeppelin-web/src/components/websocketEvents/websocketMsg.service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,12 @@ angular.module('zeppelinWebApp').service('websocketMsgSrv', function($rootScope,
});
},

getNoteRevision: function(noteId, revision) {
getNoteByRevision: function(noteId, revisionId) {
websocketEvents.sendNewEvent({
op: 'NOTE_REVISION',
data: {
noteId: noteId,
revision: revision
revisionId: revisionId
}
});
},
Expand Down
6 changes: 3 additions & 3 deletions zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,14 +358,14 @@ public Revision checkpointNote(String noteId, String checkpointMessage,
return notebookRepo.checkpoint(noteId, checkpointMessage, subject);
}

public List<NotebookRepo.Revision> listRevisionHistory(String noteId,
public List<Revision> listRevisionHistory(String noteId,
AuthenticationInfo subject) {
return notebookRepo.revisionHistory(noteId, subject);
}

public Note getNoteRevision(String noteId, Revision revision, AuthenticationInfo subject)
public Note getNoteRevision(String noteId, String revisionId, AuthenticationInfo subject)

@bzz bzz Aug 16, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we it's better to keep this and to change to getNoteByRevision ?

throws IOException {
return notebookRepo.get(noteId, revision, subject);
return notebookRepo.get(noteId, revisionId, subject);
}

@SuppressWarnings("rawtypes")
Expand Down
2 changes: 1 addition & 1 deletion zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
}

@Override
public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException {
public Note get(String noteId, String revId, AuthenticationInfo subject) throws IOException {
// Auto-generated method stub
return null;
}
Expand Down
6 changes: 3 additions & 3 deletions zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public Revision checkpoint(String pattern, String commitMessage, AuthenticationI
* 4. apply stash on top and remove it
*/
@Override
public synchronized Note get(String noteId, Revision rev, AuthenticationInfo subject)
public synchronized Note get(String noteId, String revId, AuthenticationInfo subject)
throws IOException {
Note note = null;
RevCommit stash = null;
Expand All @@ -123,7 +123,7 @@ public synchronized Note get(String noteId, Revision rev, AuthenticationInfo sub
}
ObjectId head = git.getRepository().resolve(Constants.HEAD);
// checkout to target revision
git.checkout().setStartPoint(rev.id).addPath(noteId).call();
git.checkout().setStartPoint(revId).addPath(noteId).call();
// get the note
note = super.get(noteId, subject);
// checkout back to head
Expand All @@ -137,7 +137,7 @@ public synchronized Note get(String noteId, Revision rev, AuthenticationInfo sub
stashes.size());
}
} catch (GitAPIException e) {
LOG.error("Failed to return note from revision \"{}\"", rev.message, e);
LOG.error("Failed to return note from revision \"{}\"", revId, e);
}
return note;
}
Expand Down
2 changes: 1 addition & 1 deletion zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public interface NotebookRepo {
* @return a Notebook
* @throws IOException
*/
@ZeppelinApi public Note get(String noteId, Revision rev, AuthenticationInfo subject)
@ZeppelinApi public Note get(String noteId, String revId, AuthenticationInfo subject)
throws IOException;

/**
Expand Down
6 changes: 3 additions & 3 deletions zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,12 +355,12 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
}

@Override
public Note get(String noteId, Revision rev, AuthenticationInfo subject) {
public Note get(String noteId, String revId, AuthenticationInfo subject) {
Note revisionNote = null;
try {
revisionNote = getRepo(0).get(noteId, rev, subject);
revisionNote = getRepo(0).get(noteId, revId, subject);
} catch (IOException e) {
LOG.error("Failed to get revision {} of note {}", rev.id, noteId, e);
LOG.error("Failed to get revision {} of note {}", revId, noteId, e);
}
return revisionNote;
}
Expand Down
2 changes: 1 addition & 1 deletion zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
}

@Override
public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException {
public Note get(String noteId, String revId, AuthenticationInfo subject) throws IOException {
// Auto-generated method stub
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
}

@Override
public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException {
public Note get(String noteId, String revId, AuthenticationInfo subject) throws IOException {
// Auto-generated method stub
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion ...-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
}

@Override
public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException {
public Note get(String noteId, String revId, AuthenticationInfo subject) throws IOException {
// Auto-generated method stub
return null;
}
Expand Down
8 changes: 4 additions & 4 deletions zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ public void getRevisionTest() throws IOException {
assertThat(paragraphCount_2).isEqualTo(paragraphCount_1 + 1);

// get note from revision 1
Note noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1, null);
Note noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1.id, null);
assertThat(noteRevision_1.getParagraphs().size()).isEqualTo(paragraphCount_1);

// get current note
Expand All @@ -252,7 +252,7 @@ public void getRevisionTest() throws IOException {
assertThat(paragraphCount_3).isEqualTo(paragraphCount_2 + 1);

// get revision 1 again
noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1, null);
noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1.id, null);
assertThat(noteRevision_1.getParagraphs().size()).isEqualTo(paragraphCount_1);

// check that note is unchanged
Expand Down Expand Up @@ -287,7 +287,7 @@ public void getRevisionFailTest() throws IOException {
int paragraphCount_2 = note.getParagraphs().size();

// get note from revision 1
Note noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1, null);
Note noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1.id, null);
assertThat(noteRevision_1.getParagraphs().size()).isEqualTo(paragraphCount_1);

// get current note
Expand All @@ -296,7 +296,7 @@ public void getRevisionFailTest() throws IOException {

// test for absent revision
Revision absentRevision = new Revision("absentId", StringUtils.EMPTY, 0);
note = notebookRepo.get(TEST_NOTE_ID, absentRevision, null);
note = notebookRepo.get(TEST_NOTE_ID, absentRevision.id, null);
assertThat(note).isNull();
}

Expand Down