Skip to content
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ public static enum OP {
ANGULAR_OBJECT_UPDATE, // [s-c] add/update angular object
ANGULAR_OBJECT_REMOVE, // [s-c] add angular object del

ANGULAR_OBJECT_UPDATED // [c-s] angular object value updated
ANGULAR_OBJECT_UPDATED, // [c-s] angular object value updated

CHECKPOINT_NOTEBOOK // [c-s] checkpoint notebook to storage repository
// @param noteId
// @param checkpointName
}

public OP op;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ public void onMessage(NotebookSocket conn, String msg) {
case ANGULAR_OBJECT_UPDATED:
angularObjectUpdated(conn, notebook, messagereceived);
break;
case CHECKPOINT_NOTEBOOK:
checkpointNotebook(conn, notebook, messagereceived);
break;
default:
broadcastNoteList();
break;
Expand Down Expand Up @@ -748,6 +751,13 @@ private void runParagraph(NotebookSocket conn, Notebook notebook,
}
}

private void checkpointNotebook(NotebookSocket conn, Notebook notebook,
Message fromMessage) throws IOException {
String noteId = (String) fromMessage.get("noteId");
String gitCommitMessage = (String) fromMessage.get("gitCommitMessage");
notebook.checkpointNote(noteId, gitCommitMessage);
}

/**
* Need description here.
*
Expand Down
27 changes: 27 additions & 0 deletions zeppelin-web/src/app/notebook/notebook-actionBar.html
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,33 @@ <h3>
tooltip-placement="bottom" tooltip="Export the notebook">
<i class="fa fa-download"></i>
</button>
<ul class="dropdown-menu" role="menu" style="width:250px">
<li>
<div class="cron-preset-container">
Git menu

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.

Can we make this dropdown menu independent from git?
Remove 'git' from Icons, descriptions..

<div>
<input type="text"
dropdown-input
placeholder="commit message"
id="note.checkpoint.message"
ng-model="note.checkpoint.message"/>
<button type="button"
class="btn btn-default btn-xs"
ng-hide="viewOnly"
ng-click="checkpointNotebook(note.checkpoint.message)"
tooltip-placement="bottom" tooltip="Commit the notebook">Commit
</button>
</div>
</div>
</li>
</ul>
<button type="button"
class="btn btn-default btn-xs dropdown-toggle"
ng-hide="viewOnly"
data-toggle="dropdown"
tooltip-placement="bottom" tooltip="Git menu">
<i class="fa fa-git"></i>
</button>
</span>

<span ng-hide="viewOnly">
Expand Down
15 changes: 15 additions & 0 deletions zeppelin-web/src/app/notebook/notebook.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,21 @@ angular.module('zeppelinWebApp').controller('NotebookCtrl',
});
};

// checkpoint/commit notebook
$scope.checkpointNotebook = function(commitMessage) {
BootstrapDialog.confirm({
closable: true,
title: '',
message: 'Commit notebook to local Git repository?',

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.

This one, too. 'Git' should be removed.

callback: function(result) {
if (result) {
websocketMsgSrv.checkpointNotebook($routeParams.noteId, commitMessage);
}
}
});
document.getElementById('note.checkpoint.message').value='';
};

$scope.runNote = function() {
BootstrapDialog.confirm({
closable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,16 @@ angular.module('zeppelinWebApp').service('websocketMsgSrv', function($rootScope,
});
},

checkpointNotebook: function(noteId, gitCommitMessage) {
websocketEvents.sendNewEvent({
op: 'CHECKPOINT_NOTEBOOK',
data: {
noteId: noteId,
gitCommitMessage: gitCommitMessage

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.

Can we use just 'message' or 'commitMessage' instead of 'gitCommitMessage' for the field name?

}
});
},

isConnected: function(){
return websocketEvents.isConnected();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ public void removeNote(String id) {
}
}

public void checkpointNote(String noteId, String checkpointMessage) throws IOException {
notebookRepo.checkpoint(noteId, checkpointMessage);
}

@SuppressWarnings("rawtypes")
private Note loadNoteFromRepo(String id) {
Note note = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,28 +61,34 @@ public GitNotebookRepo(ZeppelinConfiguration conf) throws IOException {
localRepo.create();
}
git = new Git(localRepo);
maybeAddAndCommit(".");
checkpoint(".", "initialization commit");

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.

I think this shouldn't be executed when Git repo already exists. Isn't it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

right, that's more friendly. Actually I changed it not to commit even at first start when initializing Git repo. So on first start all the notebooks will be unstaged, and user will explicitly commit whenever needed.

}

@Override
public synchronized void save(Note note) throws IOException {
super.save(note);
maybeAddAndCommit(note.getId());
}

private void maybeAddAndCommit(String pattern) {
/* implemented as git add+commit
* @param pattern is the noteId
* @param commitMessage is a commit message (checkpoint name)
* (non-Javadoc)
* @see org.apache.zeppelin.notebook.repo.VFSNotebookRepo#checkpoint(String, String)
*/
@Override
public void checkpoint(String pattern, String commitMessage) {
try {
List<DiffEntry> gitDiff = git.diff().call();
if (!gitDiff.isEmpty()) {
LOG.debug("Changes found for pattern '{}': {}", pattern, gitDiff);
DirCache added = git.add().addFilepattern(pattern).call();
LOG.debug("{} changes are about to be commited", added.getEntryCount());
git.commit().setMessage("Updated " + pattern).call();
git.commit().setMessage(commitMessage).call();
} else {
LOG.debug("No changes found {}", pattern);
}
} catch (GitAPIException e) {
LOG.error("Faild to add+comit {} to Git", pattern, e);
LOG.error("Failed to add+comit {} to Git", pattern, e);
}
}

Expand All @@ -100,7 +106,7 @@ public List<Rev> history(String noteId) {
Iterable<RevCommit> logs = git.log().addPath(noteId).call();
for (RevCommit log: logs) {
history.add(new Rev(log.getName(), log.getCommitTime()));
LOG.debug(" - ({},{})", log.getName(), log.getCommitTime());
LOG.debug(" - ({},{},{})", log.getName(), log.getCommitTime(), log.getFullMessage());
}
} catch (GitAPIException e) {
LOG.error("Failed to get logs for {}", noteId, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,9 @@ public interface NotebookRepo {
* Release any underlying resources
*/
public void close();

/**
* chekpoint (versioning) for notebooks (optional)
*/
public void checkpoint(String noteId, String checkPointName) throws IOException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ int getMaxRepoNum() {

NotebookRepo getRepo(int repoIndex) throws IOException {
if (repoIndex < 0 || repoIndex >= getRepoCount()) {
throw new IOException("Storage repo index is out of range");
throw new IOException("Requested storage index " + repoIndex
+ " isn't initialized," + " repository count is " + getRepoCount());
}
return repos.get(repoIndex);
}
Expand Down Expand Up @@ -351,4 +352,27 @@ public void close() {
}
}

//checkpoint to all available storages
@Override
public void checkpoint(String noteId, String checkPointName) throws IOException {
int repoCount = getRepoCount();
int errorCount = 0;
String errorMessage = "";
for (int i = 0; i < Math.min(repoCount, getMaxRepoNum()); i++) {
try {
getRepo(i).checkpoint(noteId, checkPointName);
} catch (IOException e) {
LOG.warn("Couldn't checkpoint in {} storage with index {} for note {}",
getRepo(i).getClass().toString(), i, noteId);
errorMessage += "Error on storage class " + getRepo(i).getClass().toString() +
" with index " + i + " : " + e.getMessage() + "\n";
errorCount++;
}
}
// throw exception if failed to commit for all initialized repos
if (errorCount == Math.min(repoCount, getMaxRepoNum())) {
throw new IOException(errorMessage);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,10 @@ public void remove(String noteId) throws IOException {
public void close() {
//no-op
}

@Override
public void checkpoint(String noteId, String checkPointName) throws IOException {
// no-op
LOG.info("Checkpoint feature isn't suported in {}", this.getClass().toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,10 @@ public void close() {
//no-op
}

@Override
public void checkpoint(String noteId, String checkPointName) throws IOException {
// no-op
logger.info("Checkpoint feature isn't suported in {}", this.getClass().toString());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@
import java.io.File;
import java.io.IOException;
import java.util.List;
import java.util.Map;

import org.apache.commons.io.FileUtils;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
import org.apache.zeppelin.interpreter.mock.MockInterpreter1;
import org.apache.zeppelin.interpreter.mock.MockInterpreter2;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.NoteInfo;
import org.apache.zeppelin.notebook.Paragraph;
import org.apache.zeppelin.notebook.repo.NotebookRepoVersioned.Rev;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
Expand Down Expand Up @@ -112,4 +116,39 @@ public void showNotebookHistory() throws GitAPIException, IOException {
assertThat(testNotebookHistory).isNotEmpty();
}

@Test
public void addCheckpoint() throws IOException {
// initial checks
notebookRepo = new GitNotebookRepo(conf);
assertThat(notebookRepo.list()).isNotEmpty();
assertThat(containsNote(notebookRepo.list(), TEST_NOTE_ID)).isTrue();
List<Rev> notebookHistoryBefore = notebookRepo.history(TEST_NOTE_ID);
assertThat(notebookHistoryBefore).isNotEmpty();
int initialCount = notebookHistoryBefore.size();

// add changes to note
Note note = notebookRepo.get(TEST_NOTE_ID);
Paragraph p = note.addParagraph();
Map<String, Object> config = p.getConfig();
config.put("enabled", true);
p.setConfig(config);
p.setText("%md checkpoint test text");

// save and checkpoint note
notebookRepo.save(note);
notebookRepo.checkpoint(TEST_NOTE_ID, "test commit-mesage");

// see if commit is added
List<Rev> notebookHistoryAfter = notebookRepo.history(TEST_NOTE_ID);
assertThat(notebookHistoryAfter.size()).isEqualTo(initialCount + 1);
}

private boolean containsNote(List<NoteInfo> notes, String noteId) {
for (NoteInfo note: notes) {
if (note.getId().equals(noteId)) {
return true;
}
}
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.zeppelin.notebook.repo;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -45,6 +46,7 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.quartz.SchedulerException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -59,6 +61,7 @@ public class NotebookRepoSyncTest implements JobListenerFactory {
private Notebook notebookSync;
private NotebookRepoSync notebookRepoSync;
private InterpreterFactory factory;
private SearchService search;
private static final Logger LOG = LoggerFactory.getLogger(NotebookRepoSyncTest.class);

@Before
Expand Down Expand Up @@ -89,7 +92,7 @@ public void setUp() throws Exception {

factory = new InterpreterFactory(conf, new InterpreterOption(false), null, null);

SearchService search = mock(SearchService.class);
search = mock(SearchService.class);
notebookRepoSync = new NotebookRepoSync(conf);
notebookSync = new Notebook(conf, notebookRepoSync, schedulerFactory, factory, this, search);
}
Expand Down Expand Up @@ -210,6 +213,44 @@ public void testSyncOnReloadedList() throws IOException {
assertEquals(1, notebookRepoSync.list(1).size());
}

@Test
public void testCheckpointOneStorage() throws IOException, SchedulerException {
System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE.getVarName(), "org.apache.zeppelin.notebook.repo.GitNotebookRepo");
ZeppelinConfiguration vConf = ZeppelinConfiguration.create();

NotebookRepoSync vRepoSync = new NotebookRepoSync(vConf);
Notebook vNotebookSync = new Notebook(vConf, vRepoSync, schedulerFactory, factory, this, search);

// one git versioned storage initialized
assertThat(vRepoSync.getRepoCount()).isEqualTo(1);
assertThat(vRepoSync.getRepo(0)).isInstanceOf(GitNotebookRepo.class);

GitNotebookRepo gitRepo = (GitNotebookRepo) vRepoSync.getRepo(0);

// no notes
assertThat(vRepoSync.list().size()).isEqualTo(0);
// create note
Note note = vNotebookSync.createNote();
assertThat(vRepoSync.list().size()).isEqualTo(1);

String noteId = vRepoSync.list().get(0).getId();
// first checkpoint
vRepoSync.checkpoint(noteId, "checkpoint message");
int vCount = gitRepo.history(noteId).size();
assertThat(vCount).isEqualTo(1);

Paragraph p = note.addParagraph();
Map<String, Object> config = p.getConfig();
config.put("enabled", true);
p.setConfig(config);
p.setText("%md checkpoint test");

// save and checkpoint again
vRepoSync.save(note);
vRepoSync.checkpoint(noteId, "checkpoint message 2");
assertThat(gitRepo.history(noteId).size()).isEqualTo(vCount + 1);
}

static void delete(File file){
if(file.isFile()) file.delete();
else if(file.isDirectory()){
Expand Down