You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by AhyoungRyu <gi...@git.apache.org> on 2016/07/11 10:12:25 UTC

[GitHub] zeppelin pull request #1162: [ZEPPELIN-909] Apply new mechanism to Elasticse...

GitHub user AhyoungRyu opened a pull request:

    https://github.com/apache/zeppelin/pull/1162

    [ZEPPELIN-909] Apply new mechanism to Elasticsearch interpreter

    ### What is this PR for?
    This PR applies the [new interpreter registration mechanism](https://issues.apache.org/jira/browse/ZEPPELIN-804) to Elasticsearch interpreter.
    
    ### What type of PR is it?
    Improvement
    
    ### Todos
    * [x] - Remove static property definition code lines in `ElasticsearchInterpreter.java` and create `interpreter-setting.json` under `elasticsearch/src/main/resources`
    
    ### What is the Jira issue?
    [ZEPPELIN-909](https://issues.apache.org/jira/browse/ZEPPELIN-909)
    
    ### How should this be tested?
    1. apply this patch
    2. rm -r `interpreter/elasticsearch` and rm `conf/interpreter.json`
    3. build source
    4. bin/zeppelin-daemon.sh start
    5. Run a paragraph with elasticsearch commands 
    
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/AhyoungRyu/zeppelin ZEPPELIN-909

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1162.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1162
    
----
commit f1c11565c7f75f9de0f9cc997385c4237e934030
Author: AhyoungRyu <fb...@hanmail.net>
Date:   2016-07-11T04:45:15Z

    Apply new mechanism to Elasticsearch interpreter

commit 76ac45d69626fe2260130afe5c36c2f390f152ba
Author: AhyoungRyu <fb...@hanmail.net>
Date:   2016-07-11T04:46:38Z

    Remove static block

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin pull request #1162: [ZEPPELIN-909] Apply new mechanism to Elasticse...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/zeppelin/pull/1162


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1162: [ZEPPELIN-909] Apply new mechanism to Elasticsearch in...

Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:

    https://github.com/apache/zeppelin/pull/1162
  
    Current CI failure:
    
    ```
    org.apache.zeppelin.elasticsearch.ElasticsearchInterpreterTest
    org.apache.zeppelin.elasticsearch.ElasticsearchInterpreterTest  Time elapsed: 8.15 sec  <<< ERROR!
    java.lang.NumberFormatException: null
    	at java.lang.Integer.parseInt(Integer.java:454)
    	at java.lang.Integer.parseInt(Integer.java:527)
    	at org.apache.zeppelin.elasticsearch.ElasticsearchInterpreter.<init>(ElasticsearchInterpreter.java:118)
    	at org.apache.zeppelin.elasticsearch.ElasticsearchInterpreterTest.populate(ElasticsearchInterpreterTest.java:99)
    
    
    Results :
    
    Tests in error: 
      ElasticsearchInterpreterTest.populate:99 ยป NumberFormat null
    
    Tests run: 1, Failures: 0, Errors: 1, Skipped: 0
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1162: [ZEPPELIN-909] Apply new mechanism to Elasticsearch in...

Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:

    https://github.com/apache/zeppelin/pull/1162
  
    Thank you for pointing out Lens case!
    
    May be we could extract this error handling logic to the method and re-use it everywhere, so the client code would look like:
    
    ```java
    this.resultSize = parseIntegerOrDefault(getProperty(ELASTICSEARCH_RESULT_SIZE), 10);
    ```
    
    and the API
    
    ```java
    class Interpreter {
      protected Integer parseIntegerOrDefault(String str, Integer defaultValue) { }
    }
    ```
    
    This can be done in a separate PR though and include refactoring of all existing interpreters (this, Lense, Python, etc). 
    
    What do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1162: [ZEPPELIN-909] Apply new mechanism to Elasticsearch in...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/1162
  
    @bzz CI is green now :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1162: [ZEPPELIN-909] Apply new mechanism to Elasticsearch in...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/1162
  
    @bzz Sure that make sense and thanks for clarifying it. Let me do that in other PR then :) 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---