Skip to content

Commit

Permalink
[ZEPPELIN-1366] Removed legacy JDBC alias
Browse files Browse the repository at this point in the history
### What is this PR for?
Removing old JDBC sugar

### What type of PR is it?
[Feature]

### Todos
* [x] - Removed codes to check jdbc alias enabled

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-1366

### How should this be tested?
* No longer available
  * `%jdbc(mysql)` -> `%mysql`

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? Yes
* Does this needs documentation? Yes, but I don't know where the proper location is.

Author: Jongyoul Lee <jongyoul@gmail.com>

Closes #1360 from jongyoul/ZEPPELIN-1366 and squashes the following commits:

f9df86e [Jongyoul Lee] Changed description
302825c [Jongyoul Lee] Removed effectiveText and related test
929bad2 [Jongyoul Lee] Updated docs for breaking changes
e52521f [Jongyoul Lee] Removed test for jdbc sugar
9f46bbd [Jongyoul Lee] Resolved codes conflicted
  • Loading branch information
jongyoul committed Sep 2, 2016
1 parent 51b8792 commit fe3dbdb
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 90 deletions.
3 changes: 2 additions & 1 deletion docs/install/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,5 @@ So, copying `notebook` and `conf` directory should be enough.

### Upgrading from Zeppelin 0.6 to 0.7

- From 0.7, we don't use `ZEPPELIN_JAVA_OPTS` as default value of `ZEPPELIN_INTP_JAVA_OPTS` and also the same for `ZEPPELIN_MEM`/`ZEPPELIN_INTP_MEM`. If user want to configure the jvm opts of interpreter process, please set `ZEPPELIN_INTP_JAVA_OPTS` and `ZEPPELIN_INTP_MEM` explicitly.
- From 0.7, we don't use `ZEPPELIN_JAVA_OPTS` as default value of `ZEPPELIN_INTP_JAVA_OPTS` and also the same for `ZEPPELIN_MEM`/`ZEPPELIN_INTP_MEM`. If user want to configure the jvm opts of interpreter process, please set `ZEPPELIN_INTP_JAVA_OPTS` and `ZEPPELIN_INTP_MEM` explicitly.
- Mapping from `%jdbc(prefix)` to `%prefix` is no longer available. Instead, you can use %[interpreter alias] with multiple interpreter setttings on GUI.
Original file line number Diff line number Diff line change
Expand Up @@ -427,10 +427,6 @@ public String getWebsocketMaxTextMessageSize() {
return getString(ConfVars.ZEPPELIN_WEBSOCKET_MAX_TEXT_MESSAGE_SIZE);
}

public boolean getUseJdbcAlias() {
return getBoolean(ConfVars.ZEPPELIN_USE_JDBC_ALIAS);
}

public Map<String, String> dumpConfigurations(ZeppelinConfiguration conf,
ConfigurationKeyPredicate predicate) {
Map<String, String> configurations = new HashMap<>();
Expand Down Expand Up @@ -557,9 +553,7 @@ public static enum ConfVars {
ZEPPELIN_ALLOWED_ORIGINS("zeppelin.server.allowed.origins", "*"),
ZEPPELIN_ANONYMOUS_ALLOWED("zeppelin.anonymous.allowed", true),
ZEPPELIN_CREDENTIALS_PERSIST("zeppelin.credentials.persist", true),
ZEPPELIN_WEBSOCKET_MAX_TEXT_MESSAGE_SIZE("zeppelin.websocket.max.text.message.size", "1024000"),
ZEPPELIN_USE_JDBC_ALIAS("zeppelin.use.jdbc.alias", true);

ZEPPELIN_WEBSOCKET_MAX_TEXT_MESSAGE_SIZE("zeppelin.websocket.max.text.message.size", "1024000");

private String varName;
@SuppressWarnings("rawtypes")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,8 @@ public void moveParagraph(String paragraphId, int index, boolean throwWhenIndexI

if (index < 0 || index >= paragraphs.size()) {
if (throwWhenIndexIsOutOfBound) {
throw new IndexOutOfBoundsException("paragraph size is " + paragraphs.size() +
" , index is " + index);
throw new IndexOutOfBoundsException(
"paragraph size is " + paragraphs.size() + " , index is " + index);
} else {
return;
}
Expand Down Expand Up @@ -448,7 +448,7 @@ public Map<String, String> generateSingleParagraphInfo(String paragraphId) {
return new HashMap<>();
}
}

private Map<String, String> populatePragraphInfo(Paragraph p) {
Map<String, String> info = new HashMap<>();
info.put("id", p.getId());
Expand Down Expand Up @@ -496,27 +496,15 @@ public void run(String paragraphId) {
p.setListener(jobListenerFactory.getParagraphJobListener(this));
String requiredReplName = p.getRequiredReplName();
Interpreter intp = factory.getInterpreter(getId(), requiredReplName);

if (intp == null) {
// TODO(jongyoul): Make "%jdbc" configurable from JdbcInterpreter
if (conf.getUseJdbcAlias() && null != (intp = factory.getInterpreter(getId(), "jdbc"))) {
String pText = p.getText().replaceFirst(requiredReplName, "jdbc(" + requiredReplName + ")");
logger.debug("New paragraph: {}", pText);
p.setEffectiveText(pText);
} else {
String intpExceptionMsg = format("%s",
p.getJobName()
+ "'s Interpreter "
+ requiredReplName + " not found"
);
InterpreterException intpException = new InterpreterException(intpExceptionMsg);
InterpreterResult intpResult = new InterpreterResult(
InterpreterResult.Code.ERROR, intpException.getMessage()
);
p.setReturn(intpResult, intpException);
p.setStatus(Job.Status.ERROR);
throw intpException;
}
String intpExceptionMsg =
p.getJobName() + "'s Interpreter " + requiredReplName + " not found";
InterpreterException intpException = new InterpreterException(intpExceptionMsg);
InterpreterResult intpResult =
new InterpreterResult(InterpreterResult.Code.ERROR, intpException.getMessage());
p.setReturn(intpResult, intpException);
p.setStatus(Job.Status.ERROR);
throw intpException;
}
if (p.getConfig().get("enabled") == null || (Boolean) p.getConfig().get("enabled")) {
intp.getScheduler().submit(p);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public class Paragraph extends Job implements Serializable, Cloneable {
private transient InterpreterFactory factory;
private transient Note note;
private transient AuthenticationInfo authenticationInfo;
private transient String effectiveText;

String title;
String text;
Expand Down Expand Up @@ -114,14 +113,6 @@ public void setText(String newText) {
this.dateUpdated = new Date();
}

public void setEffectiveText(String effectiveText) {
this.effectiveText = effectiveText;
}

public String getEffectiveText() {
return effectiveText;
}

public AuthenticationInfo getAuthenticationInfo() {
return authenticationInfo;
}
Expand Down Expand Up @@ -153,7 +144,7 @@ public boolean isEnabled() {
}

public String getRequiredReplName() {
return getRequiredReplName(null != effectiveText ? effectiveText : text);
return getRequiredReplName(text);
}

public static String getRequiredReplName(String text) {
Expand Down Expand Up @@ -182,7 +173,7 @@ public static String getRequiredReplName(String text) {
}

public String getScriptBody() {
return getScriptBody(null != effectiveText ? effectiveText : text);
return getScriptBody(text);
}

public static String getScriptBody(String text) {
Expand Down Expand Up @@ -359,7 +350,6 @@ protected Object jobRun() throws Throwable {
}
} finally {
InterpreterContext.remove();
effectiveText = null;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,27 +82,6 @@ public void runNormalTest() {
assertEquals("Paragraph text", pText, pCaptor.getValue().getText());
}

@Test
public void runJdbcTest() {
when(interpreterFactory.getInterpreter(anyString(), eq("mysql"))).thenReturn(null);
when(interpreterFactory.getInterpreter(anyString(), eq("jdbc"))).thenReturn(interpreter);
when(interpreter.getScheduler()).thenReturn(scheduler);

String pText = "%mysql show databases";

Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials, noteEventListener);
Paragraph p = note.addParagraph();
p.setText(pText);
note.run(p.getId());

ArgumentCaptor<Paragraph> pCaptor = ArgumentCaptor.forClass(Paragraph.class);
verify(scheduler, only()).submit(pCaptor.capture());
verify(interpreterFactory, times(2)).getInterpreter(anyString(), anyString());

assertEquals("Change paragraph text", "%jdbc(mysql) show databases", pCaptor.getValue().getEffectiveText());
assertEquals("Change paragraph text", pText, pCaptor.getValue().getText());
}

@Test
public void putDefaultReplNameIfInterpreterSettingAbsent() {
when(interpreterFactory.getDefaultInterpreterSetting(anyString()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,32 +73,6 @@ public void replNameEndsWithWhitespace() {
assertEquals("md", Paragraph.getRequiredReplName(text));
}

@Test
public void effectiveTextTest() {
InterpreterFactory interpreterFactory = mock(InterpreterFactory.class);
Interpreter interpreter = mock(Interpreter.class);
Note note = mock(Note.class);

Paragraph p = new Paragraph("paragraph", note, null, interpreterFactory);
p.setText("%h2 show databases");
p.setEffectiveText("%jdbc(h2) show databases");
assertEquals("Get right replName", "jdbc", p.getRequiredReplName());
assertEquals("Get right scriptBody", "(h2) show databases", p.getScriptBody());

when(interpreterFactory.getInterpreter(anyString(), eq("jdbc"))).thenReturn(interpreter);
when(interpreter.getFormType()).thenReturn(Interpreter.FormType.NATIVE);
when(note.getId()).thenReturn("noteId");

try {
p.jobRun();
} catch (Throwable throwable) {
// Do nothing
}

assertEquals("Erase effective Text", "h2", p.getRequiredReplName());
assertEquals("Erase effective Text", "show databases", p.getScriptBody());
}

@Test
public void should_extract_variable_from_angular_object_registry() throws Exception {
//Given
Expand Down

0 comments on commit fe3dbdb

Please sign in to comment.