Skip to content

Commit

Permalink
[ZEPPELIN-982] Improve interpreter completion API
Browse files Browse the repository at this point in the history
### What is this PR for?
When people implement a new interpreter, they extend [interpreter.java](https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java) as described in [here](https://zeppelin.apache.org/docs/0.6.0-SNAPSHOT/development/writingzeppelininterpreter.html). Among the several methods in [interpreter.java](https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java), [completion API](https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java#L109) enables auto-completion.

However this API is too simple compared to other project's auto-completion and hard to add more at the moment. So for the aspect of further expansion, it would be better to separate and restructure this API before the this release( 0.6.0 ).

### What type of PR is it?
Improvement

### Todos
* [x] - Create new structure : `InterpreterCompletion` in `RemoteInterpreterService.thrift` and regenerate `zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/thrift/*` files
* [x] - Change all existing `List<String> completion` -> `List<InterpreterCompletion> completion`
* [x] - Change `paragraph.controller.js` to point real `name` and `value`

### What is the Jira issue?
[ZEPPELIN-982](https://issues.apache.org/jira/browse/ZEPPELIN-982)

### How should this be tested?
Since this improvement is just API change, it should work same as before. So after applying this patch, and check whether auto-completion works well or not.

Use `. + ctrl` for auto-completion. For example,

```
%spark
sc.version
```

When after typing `sc.` and pushing  `. + ctrl` down, `version` should be shown in the auto-completion list.

### Screenshots (if appropriate)
![auto_completion](https://cloud.githubusercontent.com/assets/10060731/15952521/72937782-2e76-11e6-8246-4faf0dd77a5b.gif)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: AhyoungRyu <[email protected]>

Closes apache#984 from AhyoungRyu/ZEPPELIN-982 and squashes the following commits:

311dc29 [AhyoungRyu] Fix travis
9d384ec [AhyoungRyu] Address @minalee feedback
fdfae8f [AhyoungRyu] Address @jongyoul review
bd4f8c0 [AhyoungRyu] Remove abstract and make it return null by default
f8352c7 [AhyoungRyu] Fix travis error
43d81f6 [AhyoungRyu] Remove console.log
24912fa [AhyoungRyu] Fix type casting error in SparkInterpreter
80e295b [AhyoungRyu] Change return type
bd04c22 [AhyoungRyu] Apply new InterpreterCompletion class to all interpreter class files
c283043 [AhyoungRyu] Apply new InterpreterCompletion class under zeppelin-zengine/
dbecc51 [AhyoungRyu] Apply new InterpreterCompletion class under zeppelin-server/
6449455 [AhyoungRyu] Apply new InterpreterCompletion class under zeppelin-interpreter/
919b159 [AhyoungRyu] Add automatically generated thrift class
9e69e11 [AhyoungRyu] Change v -> v.name & v.value in front
73e374e [AhyoungRyu] Define InterpreterCompletion structure to thrift file
  • Loading branch information
AhyoungRyu authored and Leemoonsoo committed Jun 16, 2016
1 parent 048e432 commit 7b00dff
Show file tree
Hide file tree
Showing 60 changed files with 840 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -174,13 +175,13 @@ public int getProgress(InterpreterContext context) {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
String[] words = splitAndRemoveEmpty(splitAndRemoveEmpty(buf, "\n"), " ");
String lastWord = "";
if (words.length > 0) {
lastWord = words[ words.length - 1 ];
}
ArrayList<String> voices = new ArrayList<String>();
ArrayList voices = new ArrayList<>();
for (String command : keywords) {
if (command.startsWith(lastWord)) {
voices.add(command);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import alluxio.client.file.URIStatus;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.junit.*;

import alluxio.Constants;
Expand Down Expand Up @@ -76,19 +77,19 @@ public final void before() throws Exception {

@Test
public void testCompletion() {
List<String> expectedResultOne = Arrays.asList("cat", "chgrp",
List expectedResultOne = Arrays.asList("cat", "chgrp",
"chmod", "chown", "copyFromLocal", "copyToLocal", "count",
"createLineage");
List<String> expectedResultTwo = Arrays.asList("copyFromLocal",
List expectedResultTwo = Arrays.asList("copyFromLocal",
"copyToLocal", "count");
List<String> expectedResultThree = Arrays.asList("copyFromLocal", "copyToLocal");
List<String> expectedResultNone = new ArrayList<String>();

List<String> resultOne = alluxioInterpreter.completion("c", 0);
List<String> resultTwo = alluxioInterpreter.completion("co", 0);
List<String> resultThree = alluxioInterpreter.completion("copy", 0);
List<String> resultNotMatch = alluxioInterpreter.completion("notMatch", 0);
List<String> resultAll = alluxioInterpreter.completion("", 0);
List expectedResultThree = Arrays.asList("copyFromLocal", "copyToLocal");
List expectedResultNone = new ArrayList<String>();

List<InterpreterCompletion> resultOne = alluxioInterpreter.completion("c", 0);
List<InterpreterCompletion> resultTwo = alluxioInterpreter.completion("co", 0);
List<InterpreterCompletion> resultThree = alluxioInterpreter.completion("copy", 0);
List<InterpreterCompletion> resultNotMatch = alluxioInterpreter.completion("notMatch", 0);
List<InterpreterCompletion> resultAll = alluxioInterpreter.completion("", 0);

Assert.assertEquals(expectedResultOne, resultOne);
Assert.assertEquals(expectedResultTwo, resultTwo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.InterpreterResult.Type;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;

Expand Down Expand Up @@ -69,8 +70,8 @@ public int getProgress(InterpreterContext context) {
}

@Override
public List<String> completion(String buf, int cursor) {
return new LinkedList<String>();
public List<InterpreterCompletion> completion(String buf, int cursor) {
return new LinkedList<>();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.zeppelin.interpreter.InterpreterContext;
import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -139,7 +140,7 @@ public class CassandraInterpreter extends Interpreter {
public static final String LOGGING_DOWNGRADING_RETRY = "LOGGING_DOWNGRADING";
public static final String LOGGING_FALLTHROUGH_RETRY = "LOGGING_FALLTHROUGH";

public static final List<String> NO_COMPLETION = new ArrayList<>();
public static final List NO_COMPLETION = new ArrayList<>();

InterpreterLogic helper;
Cluster cluster;
Expand Down Expand Up @@ -320,7 +321,7 @@ public int getProgress(InterpreterContext context) {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return NO_COMPLETION;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.apache.zeppelin.interpreter.InterpreterContext;
import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.elasticsearch.action.delete.DeleteResponse;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.index.IndexResponse;
Expand Down Expand Up @@ -244,8 +245,8 @@ public int getProgress(InterpreterContext interpreterContext) {
}

@Override
public List<String> completion(String s, int i) {
final List<String> suggestions = new ArrayList<>();
public List<InterpreterCompletion> completion(String s, int i) {
final List suggestions = new ArrayList<>();

if (StringUtils.isEmpty(s)) {
suggestions.addAll(COMMANDS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.InterpreterResult.Type;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -165,7 +166,7 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.zeppelin.interpreter.Interpreter;
import org.apache.zeppelin.interpreter.InterpreterException;
import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;

/**
* HDFS implementation of File interpreter for Zeppelin.
Expand Down Expand Up @@ -259,9 +260,9 @@ public boolean isDirectory(String path) {


@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
logger.info("Completion request at position\t" + cursor + " in string " + buf);
final List<String> suggestions = new ArrayList<>();
final List suggestions = new ArrayList<>();
if (StringUtils.isEmpty(buf)) {
suggestions.add("ls");
suggestions.add("cd");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.InterpreterUtils;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -331,8 +332,8 @@ public int getProgress(InterpreterContext context) {
}

@Override
public List<String> completion(String buf, int cursor) {
return new LinkedList<String>();
public List<InterpreterCompletion> completion(String buf, int cursor) {
return new LinkedList<>();
}

private void startFlinkMiniCluster() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -300,7 +301,7 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.apache.zeppelin.hbase;

import org.apache.zeppelin.interpreter.*;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.jruby.embed.LocalContextScope;
Expand Down Expand Up @@ -152,7 +153,7 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.ignite.spi.discovery.tcp.ipfinder.vm.TcpDiscoveryVmIpFinder;
import org.apache.zeppelin.interpreter.*;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -342,7 +343,7 @@ public int getProgress(InterpreterContext context) {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return new LinkedList<>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -194,7 +195,7 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return new LinkedList<>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.apache.zeppelin.interpreter.InterpreterContext;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -113,7 +114,7 @@ public String apply(CharSequence seq) {
}
};

private static final List<String> NO_COMPLETION = new ArrayList<>();
private static final List<InterpreterCompletion> NO_COMPLETION = new ArrayList<>();

public JDBCInterpreter(Properties property) {
super(property);
Expand Down Expand Up @@ -438,11 +439,12 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
List<CharSequence> candidates = new ArrayList<>();
SqlCompleter sqlCompleter = propertyKeySqlCompleterMap.get(getPropertyKey(buf));
if (sqlCompleter != null && sqlCompleter.complete(buf, cursor, candidates) >= 0) {
return Lists.transform(candidates, sequenceToStringTransformer);
List completion = Lists.transform(candidates, sequenceToStringTransformer);
return completion;
} else {
return NO_COMPLETION;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.zeppelin.interpreter.InterpreterContext;
import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -112,7 +113,7 @@ public int getProgress(InterpreterContext context) {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -433,7 +434,7 @@ public int getProgress(InterpreterContext context) {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.zeppelin.livy;

import org.apache.zeppelin.interpreter.*;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -115,7 +116,7 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.zeppelin.livy;

import org.apache.zeppelin.interpreter.*;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -143,7 +144,7 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.zeppelin.livy;

import org.apache.zeppelin.interpreter.*;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -115,7 +116,7 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.zeppelin.livy;

import org.apache.zeppelin.interpreter.*;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.slf4j.Logger;
Expand Down Expand Up @@ -158,7 +159,7 @@ public Scheduler getScheduler() {
}

@Override
public List<String> completion(String buf, int cursor) {
public List<InterpreterCompletion> completion(String buf, int cursor) {
return null;
}

Expand Down
Loading

0 comments on commit 7b00dff

Please sign in to comment.