Skip to content

Conversation

@kjhong
Copy link

@kjhong kjhong commented Oct 14, 2016

What is this PR for?

As you can see in the attached screenshot image, "description" value doesn't show up in interpreter creation page. Moreover, the "+" (action button) is not working as well.

What type of PR is it?

Bug Fix

Todos

  • modification - interpreterfactory description
  • modification - interpreter page

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1461?jql=project%20%3D%20ZEPPELIN%20AND%20text%20~%20%22description%22

How should this be tested?

  1. on click to zeppelin interpreter page.

Screenshots (if appropriate)

after

2016-10-14 4 03 52

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

@AhyoungRyu
Copy link
Contributor

Thanks for your contribution @kjhong :)
Tested and it works well. Appreciate for bring it back 👍

@tae-jun
Copy link
Contributor

tae-jun commented Oct 15, 2016

👍

@AhyoungRyu
Copy link
Contributor

AhyoungRyu commented Oct 16, 2016

screen shot 2016-10-16 at 12 45 44 pm

But as you can see in the above screenshot img, most of CI tests have failed in zeppelin-web with below reason.

[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Zeppelin ........................................... SUCCESS [ 21.578 s]
[INFO] Zeppelin: Interpreter .............................. SUCCESS [ 20.072 s]
[INFO] Zeppelin: Zengine .................................. SUCCESS [ 14.516 s]
[INFO] Zeppelin: Display system apis ...................... SUCCESS [ 21.796 s]
[INFO] Zeppelin: Spark dependencies ....................... SUCCESS [ 53.112 s]
[INFO] Zeppelin: Spark .................................... SUCCESS [ 31.039 s]
[INFO] Zeppelin: Markdown interpreter ..................... SUCCESS [  0.815 s]
[INFO] Zeppelin: Angular interpreter ...................... SUCCESS [  0.308 s]
[INFO] Zeppelin: Shell interpreter ........................ SUCCESS [  0.376 s]
[INFO] Zeppelin: Livy interpreter ......................... SUCCESS [  1.186 s]
[INFO] Zeppelin: HBase interpreter ........................ SUCCESS [  9.760 s]
[INFO] Zeppelin: PostgreSQL interpreter ................... SUCCESS [  1.084 s]
[INFO] Zeppelin: JDBC interpreter ......................... SUCCESS [  1.275 s]
[INFO] Zeppelin: File System Interpreters ................. SUCCESS [  1.905 s]
[INFO] Zeppelin: Flink .................................... SUCCESS [ 15.244 s]
[INFO] Zeppelin: Apache Ignite interpreter ................ SUCCESS [  1.348 s]
[INFO] Zeppelin: Kylin interpreter ........................ SUCCESS [  0.419 s]
[INFO] Zeppelin: Python interpreter ....................... SUCCESS [  0.483 s]
[INFO] Zeppelin: Lens interpreter ......................... SUCCESS [  5.649 s]
[INFO] Zeppelin: Apache Cassandra interpreter ............. SUCCESS [01:09 min]
[INFO] Zeppelin: Elasticsearch interpreter ................ SUCCESS [  8.175 s]
[INFO] Zeppelin: BigQuery interpreter ..................... SUCCESS [  3.843 s]
[INFO] Zeppelin: Alluxio interpreter ...................... SUCCESS [  4.392 s]
[INFO] Zeppelin: web Application .......................... FAILURE [01:59 min]
[INFO] Zeppelin: Server ................................... SKIPPED
[INFO] Zeppelin: Packaging distribution ................... SKIPPED
[INFO] Zeppelin: R Interpreter ............................ SKIPPED
[INFO] Zeppelin: Scalding interpreter ..................... SKIPPED
[INFO] Zeppelin: Examples ................................. SKIPPED
[INFO] Zeppelin: Example application - Clock .............. SKIPPED
[INFO] Zeppelin: Example application - Horizontal Bar chart SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 06:49 min
[INFO] Finished at: 2016-10-14T14:05:07+00:00
[INFO] Final Memory: 155M/479M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:0.0.25:grunt (grunt build) on project zeppelin-web: Failed to run task: 'grunt build --no-color' failed. (error code 3) -> [Help 1]

@kjhong Let's re-trigger CI and see it helps or not. You can close and reopen this PR to retrigger it :)

@meenakshisekar
Copy link
Contributor

meenakshisekar commented Oct 19, 2016

@kjhong
The changes looks good to me, but here are few suggestions from my side.

  • Felt that the Description field need to be editable. How about displaying in a textArea?

  • How about Storing the description as a separate set of key value pairs ? so that it is completely readable.

    For ex:
    Currently after your changes, the json might look like
    "properties": {
    "hdfs.maxlength": "1000",
    "hdfs.user": "hdfs",
    "hdfs.maxlength-description": "Description of maxlength",
    "hdfs.user-description": "Description of username",
    },

    Instead if we store them as a different set of key value pairs the json might look like,
    "properties": {
    "hdfs.maxlength": "1000",
    "hdfs.user": "hdfs",
    },
    and
    "properties-description": {
    "hdfs.maxlength": "Description of maxlength",
    "hdfs.user": "Description of username",
    },

    which will be more readable and straight forward.

From future changes perspective, this will benefit us when

  • There is an additional field getting added other than description
  • When someone is calling the restApi (createInterpreter) of this via tools like postman the datastructure will be clearly defined and easy to use instead of complicating the values inside the same properties datastructure.

Let me know your views on these.

@jongyoul
Copy link
Member

I think this PR tries to make less change for the dealing with that problem. Actually the most right way changes a lot including backend. it also can break backward compatibility.

asfgit pushed a commit that referenced this pull request Nov 2, 2016
…eation page

### What is this PR for?
#1522 tried to fix same issue by making as least change as possible, but fixing it in backend side looks like more proper approach as #1522 (comment).

This PR fixes ZEPPELIN-1461 by changing `properties` field of `InterpreterSetting` class from `Properties` -> `Object`.
### What type of PR is it?

Bug Fix
### What is the Jira issue?

[ZEPPELIN-1461](https://issues.apache.org/jira/browse/ZEPPELIN-1461)
### Screenshots (if appropriate)

Before
![screen shot 2016-10-24 at 8 42 02 pm](https://cloud.githubusercontent.com/assets/8503346/19644395/5d20a864-9a2a-11e6-806a-8a44e44a108a.png)

After
![screen shot 2016-10-24 at 8 37 17 pm](https://cloud.githubusercontent.com/assets/8503346/19644281/d72be1ce-9a29-11e6-9b67-6d5de263b0de.png)
### Questions:
- Does the licenses files need update? no
- Is there breaking changes for older versions? no
- Does this needs documentation? no

Author: Mina Lee <[email protected]>

Closes #1559 from minahlee/ZEPPELIN-1461 and squashes the following commits:

4a278f0 [Mina Lee] Add test checking InterpreterProperty class
14a6300 [Mina Lee] Add selenium test for display description on interpreter create
4eba177 [Mina Lee] Fix order of properties in ui and java code style
1a2a41d [Mina Lee] Show description when create new interpreter
@asfgit asfgit closed this in 794c66b Nov 2, 2016
darionyaphet pushed a commit to darionyaphet/zeppelin that referenced this pull request Nov 4, 2016
…eation page

### What is this PR for?
apache#1522 tried to fix same issue by making as least change as possible, but fixing it in backend side looks like more proper approach as apache#1522 (comment).

This PR fixes ZEPPELIN-1461 by changing `properties` field of `InterpreterSetting` class from `Properties` -> `Object`.
### What type of PR is it?

Bug Fix
### What is the Jira issue?

[ZEPPELIN-1461](https://issues.apache.org/jira/browse/ZEPPELIN-1461)
### Screenshots (if appropriate)

Before
![screen shot 2016-10-24 at 8 42 02 pm](https://cloud.githubusercontent.com/assets/8503346/19644395/5d20a864-9a2a-11e6-806a-8a44e44a108a.png)

After
![screen shot 2016-10-24 at 8 37 17 pm](https://cloud.githubusercontent.com/assets/8503346/19644281/d72be1ce-9a29-11e6-9b67-6d5de263b0de.png)
### Questions:
- Does the licenses files need update? no
- Is there breaking changes for older versions? no
- Does this needs documentation? no

Author: Mina Lee <[email protected]>

Closes apache#1559 from minahlee/ZEPPELIN-1461 and squashes the following commits:

4a278f0 [Mina Lee] Add test checking InterpreterProperty class
14a6300 [Mina Lee] Add selenium test for display description on interpreter create
4eba177 [Mina Lee] Fix order of properties in ui and java code style
1a2a41d [Mina Lee] Show description when create new interpreter
darionyaphet pushed a commit to darionyaphet/zeppelin that referenced this pull request Nov 4, 2016
…eation page

### What is this PR for?
apache#1522 tried to fix same issue by making as least change as possible, but fixing it in backend side looks like more proper approach as apache#1522 (comment).

This PR fixes ZEPPELIN-1461 by changing `properties` field of `InterpreterSetting` class from `Properties` -> `Object`.
### What type of PR is it?

Bug Fix
### What is the Jira issue?

[ZEPPELIN-1461](https://issues.apache.org/jira/browse/ZEPPELIN-1461)
### Screenshots (if appropriate)

Before
![screen shot 2016-10-24 at 8 42 02 pm](https://cloud.githubusercontent.com/assets/8503346/19644395/5d20a864-9a2a-11e6-806a-8a44e44a108a.png)

After
![screen shot 2016-10-24 at 8 37 17 pm](https://cloud.githubusercontent.com/assets/8503346/19644281/d72be1ce-9a29-11e6-9b67-6d5de263b0de.png)
### Questions:
- Does the licenses files need update? no
- Is there breaking changes for older versions? no
- Does this needs documentation? no

Author: Mina Lee <[email protected]>

Closes apache#1559 from minahlee/ZEPPELIN-1461 and squashes the following commits:

4a278f0 [Mina Lee] Add test checking InterpreterProperty class
14a6300 [Mina Lee] Add selenium test for display description on interpreter create
4eba177 [Mina Lee] Fix order of properties in ui and java code style
1a2a41d [Mina Lee] Show description when create new interpreter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants