Skip to content
Closed
Show file tree
Hide file tree
Changes from 11 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 @@ -97,7 +97,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 @@ -151,6 +151,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 @@ -718,6 +721,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 @@ -156,6 +156,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 @@ -122,6 +122,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 @@ -38,6 +38,7 @@
import org.apache.zeppelin.interpreter.InterpreterSetting;
import org.apache.zeppelin.interpreter.remote.RemoteAngularObjectRegistry;
import org.apache.zeppelin.notebook.repo.NotebookRepo;
import org.apache.zeppelin.notebook.repo.NotebookRepoSync;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.apache.zeppelin.search.SearchService;
import org.quartz.CronScheduleBuilder;
Expand Down Expand Up @@ -240,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 part of 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("Updated " + pattern + ": " + commitMessage).call();

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.

How about just setMessage(commitMessage) as a commit message? is there special reason adding "Updated " + pattern in front?

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.

good point! Actually as I've checked now, pattern from setMessage isn't used anywhere else. addressed it in b0fa219

} 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 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 @@ -69,6 +69,7 @@ public NotebookRepoSync(ZeppelinConfiguration conf) throws Exception {
Constructor<?> constructor = notebookStorageClass.getConstructor(
ZeppelinConfiguration.class);
repos.add((NotebookRepo) constructor.newInstance(conf));
LOG.info("Initialized {} repository class", storageClassNames[i].trim());
}
if (getRepoCount() > 1) {
sync(0, 1);
Expand Down Expand Up @@ -190,9 +191,10 @@ int getMaxRepoNum() {
return maxRepoNum;
}

private NotebookRepo getRepo(int repoIndex) throws IOException {
private NotebookRepo getRepo(int repoIndex) {
if (repoIndex < 0 || repoIndex >= getRepoCount()) {
throw new IOException("Storage repo index is out of range");
LOG.error("Requested storage index {} isn't initialized," +
" repository count is {}", repoIndex, getRepoCount());
}
return repos.get(repoIndex);
}
Expand Down Expand Up @@ -319,4 +321,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());
}

}