Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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 @@ -587,6 +587,7 @@ public Response runParagraph(@PathParam("notebookId") String notebookId,
if (paramsForUpdating != null) {
paragraph.settings.getParams().putAll(paramsForUpdating);
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
note.setLastReplName(paragraph.getId());
note.persist(subject);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,7 @@ private void runParagraph(NotebookSocket conn, HashSet<String> userAndRoles, Not
// if it's the last paragraph, let's add a new one
boolean isTheLastParagraph = note.getLastParagraph().getId()
.equals(p.getId());
note.setLastReplName(paragraphId);
if (!Strings.isNullOrEmpty(text) && isTheLastParagraph) {
note.addParagraph();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.zeppelin.integration;


import org.apache.commons.lang.StringUtils;
import org.apache.zeppelin.AbstractZeppelinIT;
import org.apache.zeppelin.WebDriverManager;
import org.apache.zeppelin.ZeppelinITUtils;
Expand All @@ -28,14 +29,11 @@
import org.junit.Test;
import org.junit.rules.ErrorCollector;
import org.openqa.selenium.*;
import org.openqa.selenium.interactions.Action;
import org.openqa.selenium.interactions.Actions;
import org.openqa.selenium.support.ui.Select;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.File;

public class ParagraphActionsIT extends AbstractZeppelinIT {
private static final Logger LOG = LoggerFactory.getLogger(ParagraphActionsIT.class);

Expand Down Expand Up @@ -96,7 +94,7 @@ public void testCreateNewButton() throws Exception {

collector.checkThat("Paragraph is created above",
driver.findElement(By.xpath(getParagraphXPath(1) + "//div[contains(@class, 'editor')]")).getText(),
CoreMatchers.equalTo(""));
CoreMatchers.equalTo(StringUtils.EMPTY));
setTextOfParagraph(1, " this is above ");

newPara = driver.findElement(By.xpath(getParagraphXPath(2) + "//div[contains(@class,'new-paragraph')][2]"));
Expand All @@ -106,7 +104,7 @@ public void testCreateNewButton() throws Exception {

collector.checkThat("Paragraph is created below",
driver.findElement(By.xpath(getParagraphXPath(3) + "//div[contains(@class, 'editor')]")).getText(),
CoreMatchers.equalTo(""));
CoreMatchers.equalTo(StringUtils.EMPTY));
setTextOfParagraph(3, " this is below ");

collector.checkThat("The output field of paragraph1 contains",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ public void testNotebookCreateWithParagraphs() throws IOException {
assertEquals("compare note name", expectedNoteName, newNoteName);
assertEquals("initial paragraph check failed", 3, newNote.getParagraphs().size());
for (Paragraph p : newNote.getParagraphs()) {
if (StringUtils.isEmpty(p.getText())) {
if (StringUtils.isEmpty(p.getText()) ||
p.getText().trim().equals(newNote.getLastInterpreterName())) {
continue;
}
assertTrue("paragraph title check failed", p.getTitle().startsWith("title"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ angular.module('zeppelinWebApp')
$scope.editor.setTheme('ace/theme/chrome');
if ($scope.paragraphFocused) {
$scope.editor.focus();
$scope.goToLineEnd();
}

autoAdjustEditorHeight(_editor.container.id);
Expand Down Expand Up @@ -1001,6 +1002,10 @@ angular.module('zeppelinWebApp')
return false;
};

$scope.goToLineEnd = function () {
$scope.editor.navigateLineEnd();
};

$scope.$on('updateProgress', function(event, data) {
if (data.id === $scope.paragraph.id) {
$scope.currentProgress = data.progress;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@
import java.io.IOException;
import java.io.Serializable;
import java.util.*;
import java.util.concurrent.Callable;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;

import org.apache.commons.lang.StringUtils;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.display.AngularObject;
import org.apache.zeppelin.display.AngularObjectRegistry;
Expand All @@ -40,6 +41,7 @@
import org.apache.zeppelin.scheduler.JobListener;
import org.apache.zeppelin.search.SearchService;

import com.google.common.base.Optional;
import com.google.gson.Gson;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.apache.zeppelin.user.Credentials;
Expand All @@ -65,6 +67,7 @@ public class Note implements Serializable, JobListener {
private String name = "";
private String id;

private AtomicReference<String> lastReplName = new AtomicReference<>(StringUtils.EMPTY);
private transient ZeppelinConfiguration conf = ZeppelinConfiguration.create();

@SuppressWarnings("rawtypes")
Expand Down Expand Up @@ -108,6 +111,17 @@ private void generateId() {
id = IdHashes.encode(System.currentTimeMillis() + new Random().nextInt());
}

private String getDefaultInterpreterName() {
Optional<InterpreterSetting> settingOptional = replLoader.getDefaultInterpreterSetting();
return settingOptional.isPresent() ? settingOptional.get().getName() : StringUtils.EMPTY;
}

void putDefaultReplName() {
String defaultInterpreterName = getDefaultInterpreterName();
logger.info("defaultInterpreterName is '{}'", defaultInterpreterName);
lastReplName.set(defaultInterpreterName);
}

public String id() {
return id;
}
Expand Down Expand Up @@ -188,6 +202,7 @@ public Map<String, List<AngularObject>> getAngularObjects() {

public Paragraph addParagraph() {
Paragraph p = new Paragraph(this, this, replLoader);
addLastReplNameIfEmptyText(p);
synchronized (paragraphs) {
paragraphs.add(p);
}
Expand Down Expand Up @@ -231,12 +246,29 @@ public void addCloneParagraph(Paragraph srcParagraph) {
*/
public Paragraph insertParagraph(int index) {
Paragraph p = new Paragraph(this, this, replLoader);
addLastReplNameIfEmptyText(p);
synchronized (paragraphs) {
paragraphs.add(index, p);
}
return p;
}

/**
* Add Last Repl name If Paragraph has empty text
*
* @param p Paragraph
*/
private void addLastReplNameIfEmptyText(Paragraph p) {
String replName = lastReplName.get();
if (StringUtils.isEmpty(p.getText()) && StringUtils.isNotEmpty(replName)) {
p.setText(getInterpreterName(replName) + " ");
}
}

private String getInterpreterName(String replName) {
return StringUtils.isBlank(replName) ? StringUtils.EMPTY : "%" + replName;
}

/**
* Remove paragraph by id.
*
Expand Down Expand Up @@ -387,6 +419,9 @@ public List<Map<String, String>> generateParagraphsInfo (){
public void runAll() {
String cronExecutingUser = (String) getConfig().get("cronExecutingUser");
synchronized (paragraphs) {
if (!paragraphs.isEmpty()) {
setLastReplName(paragraphs.get(paragraphs.size() - 1));
}
for (Paragraph p : paragraphs) {
if (!p.isEnabled()) {
continue;
Expand Down Expand Up @@ -502,6 +537,16 @@ public void persist(AuthenticationInfo subject) throws IOException {
repo.save(this, subject);
}

private void setLastReplName(Paragraph lastParagraphStarted) {
if (StringUtils.isNotEmpty(lastParagraphStarted.getRequiredReplName())) {
lastReplName.set(lastParagraphStarted.getRequiredReplName());
}
}

public void setLastReplName(String paragraphId) {
setLastReplName(getParagraph(paragraphId));
}

/**
* Persist this note with maximum delay.
* @param maxDelaySec
Expand Down Expand Up @@ -567,6 +612,14 @@ public void setInfo(Map<String, Object> info) {
this.info = info;
}

public String getLastReplName() {
return lastReplName.get();
}

public String getLastInterpreterName() {
return getInterpreterName(getLastReplName());
}

@Override
public void beforeStatusChange(Job job, Status before, Status after) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.apache.zeppelin.notebook;

import com.google.common.base.Optional;

import java.io.IOException;
import java.util.LinkedList;
import java.util.List;
Expand Down Expand Up @@ -119,7 +121,7 @@ public Interpreter get(String replName) {

if (replName == null || replName.trim().length() == 0) {
// get default settings (first available)
InterpreterSetting defaultSettings = settings.get(0);
InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings).get();
return createOrGetInterpreterList(defaultSettings).get(0);
}

Expand Down Expand Up @@ -156,7 +158,7 @@ public Interpreter get(String replName) {
} else {
// first assume replName is 'name' of interpreter. ('groupName' is ommitted)
// search 'name' from first (default) interpreter group
InterpreterSetting defaultSetting = settings.get(0);
InterpreterSetting defaultSetting = getDefaultInterpreterSetting(settings).get();
Interpreter.RegisteredInterpreter registeredInterpreter =
Interpreter.registeredInterpreters.get(defaultSetting.getGroup() + "." + replName);
if (registeredInterpreter != null) {
Expand Down Expand Up @@ -190,4 +192,16 @@ public Interpreter get(String replName) {

return null;
}

private Optional<InterpreterSetting>
getDefaultInterpreterSetting(List<InterpreterSetting> settings) {
if (settings == null || settings.isEmpty()) {
return Optional.absent();
}
return Optional.of(settings.get(0));
}
Copy link
Member

Choose a reason for hiding this comment

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

Why do you make this method to return optional, not just returning getDefaultInterpreterSetting(getInterpreterSettings())? It looks like that Note.getDefaultInterpreterName uses this method only. If you want to check nullable list and emptiness, should you check the above method of this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh.. thank you for your comment.
I will change my code.


Optional<InterpreterSetting> getDefaultInterpreterSetting() {
return getDefaultInterpreterSetting(getInterpreterSettings());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ public Note createNote(List<String> interpreterIds, AuthenticationInfo subject)
}
if (interpreterIds != null) {
bindInterpretersToNote(note.id(), interpreterIds);
note.putDefaultReplName();
}

notebookIndex.addIndexDoc(note);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@

package org.apache.zeppelin.notebook;

import com.google.common.base.Optional;

import org.apache.commons.lang.StringUtils;
import org.apache.zeppelin.interpreter.Interpreter;
import org.apache.zeppelin.interpreter.InterpreterSetting;
import org.apache.zeppelin.notebook.repo.NotebookRepo;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.search.SearchService;
Expand All @@ -26,6 +30,7 @@
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.runners.MockitoJUnitRunner;

import static org.junit.Assert.*;
Expand Down Expand Up @@ -91,4 +96,73 @@ public void runJdbcTest() {
assertEquals("Change paragraph text", "%jdbc(mysql) show databases", pCaptor.getValue().getEffectiveText());
assertEquals("Change paragraph text", pText, pCaptor.getValue().getText());
}

@Test
public void putDefaultReplNameIfInterpreterSettingAbsent() {
when(replLoader.getDefaultInterpreterSetting())
.thenReturn(Optional.<InterpreterSetting>absent());

Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials);
note.putDefaultReplName();

assertEquals(StringUtils.EMPTY, note.getLastReplName());
assertEquals(StringUtils.EMPTY, note.getLastInterpreterName());
}

@Test
public void putDefaultReplNameIfInterpreterSettingPresent() {
InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class);
when(interpreterSetting.getName()).thenReturn("spark");
when(replLoader.getDefaultInterpreterSetting())
.thenReturn(Optional.of(interpreterSetting));

Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials);
note.putDefaultReplName();

assertEquals("spark", note.getLastReplName());
assertEquals("%spark", note.getLastInterpreterName());
}

@Test
public void addParagraphWithLastReplName() {
InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class);
when(interpreterSetting.getName()).thenReturn("spark");
when(replLoader.getDefaultInterpreterSetting())
.thenReturn(Optional.of(interpreterSetting));

Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials);
note.putDefaultReplName(); //set lastReplName

Paragraph p = note.addParagraph();

assertEquals("%spark ", p.getText());
}

@Test
public void insertParagraphWithLastReplName() {
InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class);
when(interpreterSetting.getName()).thenReturn("spark");
when(replLoader.getDefaultInterpreterSetting())
.thenReturn(Optional.of(interpreterSetting));

Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials);
note.putDefaultReplName(); //set lastReplName

Paragraph p = note.insertParagraph(note.getParagraphs().size());

assertEquals("%spark ", p.getText());
}

@Test
public void setLastReplName() {
String paragraphId = "HelloWorld";
Note note = Mockito.spy(new Note(repo, replLoader, jobListenerFactory, index, credentials));
Paragraph mockParagraph = Mockito.mock(Paragraph.class);
when(note.getParagraph(paragraphId)).thenReturn(mockParagraph);
when(mockParagraph.getRequiredReplName()).thenReturn("spark");

note.setLastReplName(paragraphId);

assertEquals("spark", note.getLastReplName());
}
}