[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...
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...
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2903 I think so, @zjffdu what do you think about this in 0.8? ---
[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...
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...
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2903 looks good! merging if no more comment ---
[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2903 Hi @felixcheung please let me know if any additional changes or adjustments are needed. The two failing tests are unrelated to the code in in this PR Thanks. ---
[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2903 Closing and reopening to trigger tests. ---
[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...
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...
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2903 Oops! I must have messed up somewhere! I've corrected the style issues now @felixcheung, and the one remaining failure appears to be a build script problem unrelated to my code. Thanks! ---
[GitHub] zeppelin issue #2903: [ZEPPELIN-3377] Passing Z variables to JDBC interprete...
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...
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...
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...
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...
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. ---
