You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@zeppelin.apache.org by sanjaydasgupta <gi...@git.apache.org> on 2018/04/02 07:07:18 UTC

[GitHub] zeppelin pull request #2903: [ZEPPELIN-3377] Passing Z variables to JDBC int...

GitHub user sanjaydasgupta opened a pull request:

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

    [ZEPPELIN-3377] Passing Z variables to JDBC interpreter

    ### What is this PR for?
    This PR enables the interpolation of ZeppelinContext objects into the paragraph text of JDBC cells. It also introduces a new interpreter-level configuration parameter named _zeppelin.jdbc.interpolation_. This new parameter is _false_ by default, and must be set to _true_ to enable object interpolation. The default value of _false_ guarantees backward compatibility for users who are not aware of the new feature.
    
    The implementation takes the same approach that was followed in [PR-2898](https://github.com/apache/zeppelin/pull/2898).
    
    I have also taken the liberty to correct a preexisting error in the description of the use of Dynamic Forms in the associated documentation (_jdbc.md_).
    
    ### What type of PR is it?
    [Feature]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/browse/ZEPPELIN-3377
    
    ### How should this be tested?
    CI Pass. 
    
    The code in this PR merely causes the JDBC interpreter to "opt-in" to the implementation already existing in the `Interpreter` base class - described in [PR-2898](https://github.com/apache/zeppelin/pull/2898). The unit tests necessary are already present in PR-2898
    
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? Yes, documentation has been added to the file _jdbc.md_. I have also taken the liberty to correct a preexisting error in the description of the use of Dynamic Forms in the associated documentation.


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

    $ git pull https://github.com/sanjaydasgupta/zeppelin zeppelin-3342-jdbc

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

    https://github.com/apache/zeppelin/pull/2903.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 #2903
    
----
commit 28688256f3b11e696c08ed10875e0596db8c0463
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-04T07:36:48Z

    ZEPPELIN-1967: Initial updates

commit b461c82569e86436239c9d14a8ae38230bc4f357
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-05T02:00:10Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit ced295cb8b27f499b739758b1b17460ab6d67e58
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-06T03:47:32Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit a3215fca058398ecbed840a69b4dbf57a2167447
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-07T05:15:11Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit f43fd9987b7a10ae41879c52096fdb99f8a28f34
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-11T05:40:21Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit 3beebce5b5fbb45b295fd5b4bf4b28f9bef8350f
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-13T11:33:09Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit 3b30ea2143813dfbc48970b1ac094fea30d7d775
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-15T10:43:53Z

    Reversing previous incorrect update

commit 1718e79646b8de4682cb1c76eb6d4a24e83b3184
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-15T14:17:49Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit 526880301877125501ea0b1b06c61fcfdc19fe60
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-18T04:41:52Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit b7ddf6ba202c2624a289aff67cfad669510ca80d
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-18T09:54:23Z

    Implementing configuration (global enable/disable interpolation) following https://github.com/apache/zeppelin/pull/2834#issuecomment-373948398

commit a1703b820e42e3956f45edf3c02aec8fc3475739
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-18T21:05:42Z

    Changes suggested in Felix Cheung's review https://github.com/apache/zeppelin/pull/2834#pullrequestreview-104805661

commit 3dd3dd879187e20a80cb202df7869c923da8a690
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-20T03:47:11Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit 1e2c87dd36dc091ca898baf8e9f178d6d1a5e600
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-21T01:18:09Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit cc3727f4ba70cf4b08b6f8e31a7c760493de82fd
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-25T15:06:12Z

    Changes due the Jeff Zhang's comments at https://github.com/apache/zeppelin/pull/2834/files/1e2c87dd36dc091ca898baf8e9f178d6d1a5e600#r176930418

commit d600d86eb7d672526c40d7068f8e7aae5de1f71e
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-25T15:49:43Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit 5f8505b468d9209be750c9615baf8ab4f3828726
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-26T01:53:28Z

    Changes due to Felix Cheung's comments at https://github.com/apache/zeppelin/pull/2834#pullrequestreview-106738198

commit 77738aae27515ba0c6683fcf0c181d5919892926
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-26T08:01:09Z

    Changes to comply with Felix Cheung's comment at https://github.com/apache/zeppelin/pull/2834#discussion_r176976263 and Jeff Zhang's subsequent clarification

commit a19e9989031636c183d3165448ee9ffa8f4358d3
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-28T15:56:38Z

    Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1967

commit 0f49867310a843d9fe56637ddab29f09bc193b4a
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-03-30T16:20:27Z

    Merge branch 'master' of https://github.com/apache/zeppelin into zeppelin-3342-hdfs

commit eb9194d20fc311b78e86b5cfc27c38e7a266923c
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-04-02T06:22:57Z

    ZEPPELIN-3377 Updates Initial Load

commit 315a9ad44cbc1a80ea4c4bbcafae160f1945c387
Author: Sanjay Dasgupta <sa...@...>
Date:   2018-04-02T06:39:03Z

    Corrected use of rlike in SQL statement

----


---

[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...

Posted by sanjaydasgupta <gi...@git.apache.org>.

Github user sanjaydasgupta commented on the issue:

    https://github.com/apache/zeppelin/pull/2903
  
    @felixcheung I've fixed the 4 issues in the documentation file jdbc.md, thanks for the very careful review.
    
    I have also added a unit test [JDBCInterpreterInterpolationTest.java](https://github.com/apache/zeppelin/pull/2903/files#diff-f8ebbd56b41630cac1cec76efe751d9d) which tests the following:
    
    1) enabling and disabling the feature by setting "zeppelin.jdbc.interpolation", including the default value of false.
    2) normal substitution of {...} patterns
    3) escaping the pattern functionality by using {{...}}
    
    Please also note that all the unit tests defined for this feature in the `Interpreter` base class do a thorough check of all possible uses and misuses of { and }. The above additional unit test is really more like an integration test to verify that the JDBC interpreter is inheriting and leveraging the base class's functionality correctly.
    
    Thanks for your very helpful review.


---

[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...

Posted by sanjaydasgupta <gi...@git.apache.org>.

Github user sanjaydasgupta commented on the issue:

    https://github.com/apache/zeppelin/pull/2903
  
    Apologies for missing the check style errors @felixcheung 
    
    The style issues in _JDBCInterpreterInterpolationTest.java_ have been removed, and all the tests now pass. The other failures in the build are unrelated to this feature.
    
    Thanks  for your review.


---

[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...

Posted by sanjaydasgupta <gi...@git.apache.org>.

Github user sanjaydasgupta commented on the issue:

    https://github.com/apache/zeppelin/pull/2903
  
    ping @felixcheung 


---

[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...

Posted by felixcheung <gi...@git.apache.org>.

Github user felixcheung commented on the issue:

    https://github.com/apache/zeppelin/pull/2903
  
    merged this to master & 0.8


---

[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...

Posted by sanjaydasgupta <gi...@git.apache.org>.

Github user sanjaydasgupta commented on the issue:

    https://github.com/apache/zeppelin/pull/2903
  
    Hi @felixcheung, can these changes be merged now? 


---

[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...

Posted by felixcheung <gi...@git.apache.org>.

Github user felixcheung commented on the issue:

    https://github.com/apache/zeppelin/pull/2903
  
    still have errors on this link https://travis-ci.org/sanjaydasgupta/zeppelin/builds/363808230?



---

[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...

Posted by sanjaydasgupta <gi...@git.apache.org>.

Github user sanjaydasgupta commented on the issue:

    https://github.com/apache/zeppelin/pull/2903
  
    Hi @felixcheung is it possible to release this into 0.8.0?
    
    Thanks


---

[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...

Posted by sanjaydasgupta <gi...@git.apache.org>.

Github user sanjaydasgupta commented on the issue:

    https://github.com/apache/zeppelin/pull/2903
  
    @felixcheung Please note [my response](https://github.com/apache/zeppelin/pull/2903#discussion_r178733031) to your earlier question about the unrelated documentation change. 
    
    Please let me know if there are any other concerns. Thanks.


---