Uploaded image for project: 'Zeppelin'
  1. Zeppelin
  2. ZEPPELIN-2807

Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967)

Details

    • New Feature
    • Status: Closed
    • Major
    • Resolution: Won't Do
    • 0.7.0, 0.8.0
    • None
    • Interpreters
    • None

    Description

      The issue https://issues.apache.org/jira/browse/ZEPPELIN-1967 requests implementation of the same functionality in different interpreters (and in different interpreter groups). But it may be simpler to implement the function separately in each interpreter of each group.

      This issue has been created to accompany an implementation for the Spark SQL interpreter.

      Attachments

        Issue Links

          Activity

            sanjay.dasgupta@gmail.com Sanjay Dasgupta created issue -
            sanjay.dasgupta@gmail.com Sanjay Dasgupta made changes -
            Field Original Value New Value
            Summary Passing Z variables to SQL Interpreter (One part of ZEPPLIN-1967) Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967)
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            GitHub user sanjaydasgupta opened a pull request:

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

            ZEPPELIN-2807 Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967)

                1. What is this PR for?
                  This feature allows embedding/interpolating Z variables into SQL statements passed to the Spark-SQL interpreter. It implements one part of the functions desired by the pre-existing issue (ZEPPELIN-1967)
                1. What type of PR is it?
                  [Improvement]
                1. Todos
            • [ ] - Task
                1. What is the Jira issue?
                  https://issues.apache.org/jira/browse/ZEPPELIN-2807
                1. How should this be tested?
                  A new test has been added to the unit tests. The attached screenshot also shows tests of the functionality.
                1. Screenshots (if appropriate)
                  ![sparksqlchange](https://user-images.githubusercontent.com/477015/28521954-a1148532-7093-11e7-85e8-293d86fb0362.png)
                1. Questions:
            • Does the licenses files need update? No
            • Is there breaking changes for older versions? No
            • Does this needs documentation? Yes, a small update to any existing documentation is ideal.

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

            $ git pull https://github.com/sanjaydasgupta/zeppelin master

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

            https://github.com/apache/zeppelin/pull/2502.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 #2502


            commit e8ba58a94743b743f2a05cef8b9755d5bf79510c
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-24T11:29:29Z

            ZEPPELIN-2807: Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967)


            githubbot ASF GitHub Bot added a comment - GitHub user sanjaydasgupta opened a pull request: https://github.com/apache/zeppelin/pull/2502 ZEPPELIN-2807 Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967 ) What is this PR for? This feature allows embedding/interpolating Z variables into SQL statements passed to the Spark-SQL interpreter. It implements one part of the functions desired by the pre-existing issue ( ZEPPELIN-1967 ) What type of PR is it? [Improvement] Todos [ ] - Task What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-2807 How should this be tested? A new test has been added to the unit tests. The attached screenshot also shows tests of the functionality. Screenshots (if appropriate) ! [sparksqlchange] ( https://user-images.githubusercontent.com/477015/28521954-a1148532-7093-11e7-85e8-293d86fb0362.png ) Questions: Does the licenses files need update? No Is there breaking changes for older versions? No Does this needs documentation? Yes, a small update to any existing documentation is ideal. You can merge this pull request into a Git repository by running: $ git pull https://github.com/sanjaydasgupta/zeppelin master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zeppelin/pull/2502.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 #2502 commit e8ba58a94743b743f2a05cef8b9755d5bf79510c Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-24T11:29:29Z ZEPPELIN-2807 : Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967 )
            Tagar Ruslan Dautkhanov made changes -
            Link This issue Parent Feature ZEPPELIN-1967 [ ZEPPELIN-1967 ]

            I am not being able to understand what is wrong with the one test that is failing on travis (https://travis-ci.org/sanjaydasgupta/zeppelin/builds/257159221).

            Any hints to help with this would be very helpful.

            Thanks for any help

            sanjay.dasgupta@gmail.com Sanjay Dasgupta added a comment - I am not being able to understand what is wrong with the one test that is failing on travis ( https://travis-ci.org/sanjaydasgupta/zeppelin/builds/257159221 ). Any hints to help with this would be very helpful. Thanks for any help
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Thank you for the observations @felixcheung.
            Will review these points and make suitable changes.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Thank you for the observations @felixcheung. Will review these points and make suitable changes.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            The choice of

            {...} for identifying parameter/variable names comes from the parent issue ZEPPELIN-1967(https://issues.apache.org/jira/browse/ZEPPELIN-1967). Though #{...}

            seems more widely used in other settings and languages, I have no opinion in the matter, and would like to request the original issue's author @Tagar to offer any comments. Thanks for your ideas and help with this.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 The choice of {...} for identifying parameter/variable names comes from the parent issue ZEPPELIN-1967 ( https://issues.apache.org/jira/browse/ZEPPELIN-1967 ). Though #{...} seems more widely used in other settings and languages, I have no opinion in the matter, and would like to request the original issue's author @Tagar to offer any comments. Thanks for your ideas and help with this.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user Tagar commented on the issue:

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

            @sanjaydasgupta great job on getting this done.
            ZEPPELIN-1967 was inspired by Jupyter so I just we should do the same as it is done there.
            So it's less confusing for users who are switching from Jupyter.
            I would suggest not to use #

            {...}

            as it'll be confusing to use this naming convention in shell interpreter, for example.
            Escaping should be done similarly like in Jupyter.
            With double curly braces –
            {{ will be escaped to single left curly brace - {
            }} will be escaped to single right curly brace - }
            Thanks!

            githubbot ASF GitHub Bot added a comment - Github user Tagar commented on the issue: https://github.com/apache/zeppelin/pull/2502 @sanjaydasgupta great job on getting this done. ZEPPELIN-1967 was inspired by Jupyter so I just we should do the same as it is done there. So it's less confusing for users who are switching from Jupyter. I would suggest not to use # {...} as it'll be confusing to use this naming convention in shell interpreter, for example. Escaping should be done similarly like in Jupyter. With double curly braces – {{ will be escaped to single left curly brace - { }} will be escaped to single right curly brace - } Thanks!
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            I would like your views @felixcheung @zjffdu @Tagar on the following approach to escaping and regex errors etc:

            I wish to suggest that no escaping is needed, as the substitution code kicks in only when 2 conditions are met: (1) the pattern

            {...} exists in the command, and (2) the variable named in {...}

            actually has a mapping in Z. If the second condition is not met, because a prior z.put(...) has not been executed for the same variable name, the original command including the

            {...} is left unchanged. Since the variable name used is expected to be meaningful, and is entirely under the end-user's control, it is always possible to avoid the need for escaping. Leaving the original command unchanged has other utilities as shown in the examples below.

            *Setup*
            ```
            val df = spark.createDataFrame(Seq((1, "one"), (3, "three"),
            (Int.MaxValue / 2, "##"), (Int.MaxValue, "####"))).toDF("id", "name")
            df.createOrReplaceTempView("name")

            z.put("n", 3)
            z.put("table", "name")
            ```
            *Trivial Case*
            The following SQL command works normally – performing variable substitution as expected.
            `%sql select * from {table} where id = {n}`
            The output has one row: (3, three)

            *Unchanged Pattern-Like Construction*
            But the following command also works – with the pattern-like structure (which is actually a regex passed to regex) left unchanged.
            `%sql select * from {table} where name rlike '#{2}'`
            The output has 2 rows: (1073741823, ##) and (2147483647, ####)

            Note that allowing substitution patterns inside quotes is an advantage, giving us a string-interpolation facility like in Scala (and other languages). Again, escaping is not required here also as long as the content of {...}

            is judiciously chosen.

            *One Last Example*
            The following example has two

            {...}

            structures, the first is part of a regex, the second is a real pattern.
            ```
            z.put("what", "e")
            %sql select * from

            {table}

            where name rlike '^.

            {3} {what}

            .$'
            ```
            The output has one row: (3, three)

            Incomplete patterns are also left unchanged, this could be an advantage for future embedded mini-languages in the same scope.

            Would like to hear your views on the this proposal to not implement escapes for embedded variable patterns. I only started looking at Zeppelin internals about 10 days back, so your insight will be very helpful.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 I would like your views @felixcheung @zjffdu @Tagar on the following approach to escaping and regex errors etc: I wish to suggest that no escaping is needed, as the substitution code kicks in only when 2 conditions are met: (1) the pattern {...} exists in the command, and (2) the variable named in {...} actually has a mapping in Z. If the second condition is not met, because a prior z.put(...) has not been executed for the same variable name, the original command including the {...} is left unchanged. Since the variable name used is expected to be meaningful, and is entirely under the end-user's control, it is always possible to avoid the need for escaping. Leaving the original command unchanged has other utilities as shown in the examples below. * Setup * ``` val df = spark.createDataFrame(Seq((1, "one"), (3, "three"), (Int.MaxValue / 2, "##"), (Int.MaxValue, "####"))).toDF("id", "name") df.createOrReplaceTempView("name") z.put("n", 3) z.put("table", "name") ``` * Trivial Case * The following SQL command works normally – performing variable substitution as expected. `%sql select * from {table} where id = {n}` The output has one row: (3, three) * Unchanged Pattern-Like Construction * But the following command also works – with the pattern-like structure (which is actually a regex passed to regex ) left unchanged. `%sql select * from {table} where name rlike '#{2}'` The output has 2 rows: (1073741823, ##) and (2147483647, ####) Note that allowing substitution patterns inside quotes is an advantage, giving us a string-interpolation facility like in Scala (and other languages). Again, escaping is not required here also as long as the content of {...} is judiciously chosen. * One Last Example * The following example has two {...} structures, the first is part of a regex, the second is a real pattern. ``` z.put("what", "e") %sql select * from {table} where name rlike '^. {3} {what} .$' ``` The output has one row: (3, three) Incomplete patterns are also left unchanged, this could be an advantage for future embedded mini-languages in the same scope. Would like to hear your views on the this proposal to not implement escapes for embedded variable patterns. I only started looking at Zeppelin internals about 10 days back, so your insight will be very helpful.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user felixcheung commented on the issue:

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

            I like what you propose, but think it might be very confusing for user if behavior changes depending on whether the variable is in the z context. for instance, it could be a shared notebook and someone might have set `what` and that would changes the outcome of the sql query example you have

            githubbot ASF GitHub Bot added a comment - Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2502 I like what you propose, but think it might be very confusing for user if behavior changes depending on whether the variable is in the z context. for instance, it could be a shared notebook and someone might have set `what` and that would changes the outcome of the sql query example you have
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Thank you for the insight. Do you think we may retain variable substitution within literal strings as in the following contrived example?

            `%sql select * from spares where part_no = 'pump-

            {manufacturer-code}

            '`

            Thanks again for your help.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Thank you for the insight. Do you think we may retain variable substitution within literal strings as in the following contrived example? `%sql select * from spares where part_no = 'pump- {manufacturer-code} '` Thanks again for your help.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user felixcheung commented on the issue:

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

            I guess that's tricky.
            like in your example, if it is in `'`, should we leave it alone, or not? `%sql select * from table where name rlike '#

            {2}

            '`

            I suppose we should first make sure the behavior is consistent in all interpreters and in all/most of token replacement

            githubbot ASF GitHub Bot added a comment - Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2502 I guess that's tricky. like in your example, if it is in `'`, should we leave it alone, or not? `%sql select * from table where name rlike '# {2} '` I suppose we should first make sure the behavior is consistent in all interpreters and in all/most of token replacement
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Agreed, thanks.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Agreed, thanks.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Hello @felixcheung @Tagar @zjffdu, this is for any further comments and suggestions. Here are the features implemented:

            1. - The substitution pattern is `

            { ... }` as suggested for uniformity with Jupyter
            2. - The substitution pattern can be escaped by using `{{ ... }}`, and such patterns are translated into `{ ... }

            ` while retaining the original contents of `{{ ... }}`
            3. - Substitution patterns inside strings (like `'part-

            {number}

            '`) are active, and will be substituted. This is also as in the Jupyter feature (@felixcheung this is different from our last exchange)

            I have expanded the unit-tests to include these cases, and have added error checks for a variety of ill-formed patterns. The following screenshots show some examples of the feature in use.

            ![fig-01](https://user-images.githubusercontent.com/477015/28742890-912ba640-7459-11e7-98dd-8960b2f76a31.png)
            ![fig-02](https://user-images.githubusercontent.com/477015/28742891-912dfc2e-7459-11e7-8a6c-3790de2ef664.png)
            ![fig-03](https://user-images.githubusercontent.com/477015/28742892-9132a9b8-7459-11e7-872d-6c496a98af5c.png)
            ![fig-04](https://user-images.githubusercontent.com/477015/28742893-913749c8-7459-11e7-903b-40b664d1cb2f.png)

            Thanks for all your pointers and ideas.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Hello @felixcheung @Tagar @zjffdu, this is for any further comments and suggestions. Here are the features implemented: 1. - The substitution pattern is ` { ... }` as suggested for uniformity with Jupyter 2. - The substitution pattern can be escaped by using `{{ ... }}`, and such patterns are translated into `{ ... } ` while retaining the original contents of `{{ ... }}` 3. - Substitution patterns inside strings (like `'part- {number} '`) are active, and will be substituted. This is also as in the Jupyter feature (@felixcheung this is different from our last exchange) I have expanded the unit-tests to include these cases, and have added error checks for a variety of ill-formed patterns. The following screenshots show some examples of the feature in use. ! [fig-01] ( https://user-images.githubusercontent.com/477015/28742890-912ba640-7459-11e7-98dd-8960b2f76a31.png ) ! [fig-02] ( https://user-images.githubusercontent.com/477015/28742891-912dfc2e-7459-11e7-8a6c-3790de2ef664.png ) ! [fig-03] ( https://user-images.githubusercontent.com/477015/28742892-9132a9b8-7459-11e7-872d-6c496a98af5c.png ) ! [fig-04] ( https://user-images.githubusercontent.com/477015/28742893-913749c8-7459-11e7-903b-40b664d1cb2f.png ) Thanks for all your pointers and ideas.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user felixcheung commented on the issue:

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

            I think it's fine to replace token within quoted string - this is probably a better, more consistent behavior anyway.

            I am concerned with having different ways to replace token in different interpreters though, as pointed out in https://github.com/apache/zeppelin/pull/2502#pullrequestreview-52283678; as adding to that we also have the `${token}` already https://zeppelin.apache.org/docs/0.7.2/manual/dynamicform.html#text-input-form

            Can you point me to how this is used in jupyter?

            githubbot ASF GitHub Bot added a comment - Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2502 I think it's fine to replace token within quoted string - this is probably a better, more consistent behavior anyway. I am concerned with having different ways to replace token in different interpreters though, as pointed out in https://github.com/apache/zeppelin/pull/2502#pullrequestreview-52283678 ; as adding to that we also have the `${token}` already https://zeppelin.apache.org/docs/0.7.2/manual/dynamicform.html#text-input-form Can you point me to how this is used in jupyter?
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Digging a little deeper to clarify the conflict ... The class `BaseZeppelinContext` uses two different key-value repositories for two different needs:

            1. *Storage of user-provided inputs entered into dynamic form fields*. These are held in an `AngularObjectRegistry` associated with the particular interpreter in effect, and are managed either (a) programmatically by the functions `z.angularBind(...)` and `z.angularUnbind(...)`, or (b) by using the `${ ... }` notation described at the link you provided (text-input-form) – this second option is the one that seems to be (but in fact is not) in conflict with the substitution proposal described here.
            2. *Programmatically assigned data intended to be shared across interpreters* (currently Spark, R, and Python). These are held in a `ResourcePool`, and are managed by the functions `z.put(...)`, and `z.get(...)`. This pull request seeks to substitute values from a `ResourcePool` into `

            { ... }` patterns in interpreter commands.

            So, as described above, these are two different facilities, and use similar but different substitution patterns: the dynamic-forms facility uses `${ ... }` while this pull request (inter-interpreter data sharing facility) proposes to use `{ ... }

            ` and `{{ ... }}` (for escaping the basic substitution pattern).

            The Jupyter Python examples quoted are drawn from the context of shell commands spawned by prefixing a command with `!` operator. In this case the

            { ... } pattern can be used to embed Python snippets (not just a variable name) into the command, and `{{ ... }}` is used for escaping the `{ ... }

            ` substitution pattern.

            I hope this clarifies the two patterns and uses.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Digging a little deeper to clarify the conflict ... The class `BaseZeppelinContext` uses two different key-value repositories for two different needs: 1. * Storage of user-provided inputs entered into dynamic form fields *. These are held in an `AngularObjectRegistry` associated with the particular interpreter in effect, and are managed either (a) programmatically by the functions `z.angularBind(...)` and `z.angularUnbind(...)`, or (b) by using the `${ ... }` notation described at the link you provided (text-input-form) – this second option is the one that seems to be (but in fact is not) in conflict with the substitution proposal described here. 2. * Programmatically assigned data intended to be shared across interpreters * (currently Spark, R, and Python). These are held in a `ResourcePool`, and are managed by the functions `z.put(...)`, and `z.get(...)`. This pull request seeks to substitute values from a `ResourcePool` into ` { ... }` patterns in interpreter commands. So, as described above, these are two different facilities, and use similar but different substitution patterns: the dynamic-forms facility uses `${ ... }` while this pull request (inter-interpreter data sharing facility) proposes to use `{ ... } ` and `{{ ... }}` (for escaping the basic substitution pattern). The Jupyter Python examples quoted are drawn from the context of shell commands spawned by prefixing a command with `!` operator. In this case the { ... } pattern can be used to embed Python snippets (not just a variable name) into the command, and `{{ ... }}` is used for escaping the `{ ... } ` substitution pattern. I hope this clarifies the two patterns and uses.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            I can have a go at creating/updating the documentation if someone can help me with any guideance on where/how to begin.

            My familiarity with Jupyter is very limited, so while I will dig around, I would also request @Tagar to help if possible.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 I can have a go at creating/updating the documentation if someone can help me with any guideance on where/how to begin. My familiarity with Jupyter is very limited, so while I will dig around, I would also request @Tagar to help if possible.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user Tagar commented on the issue:

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

            Info from Jupyter comitter @takluyver

            > It's fairly brief, but here:
            > http://ipython.readthedocs.io/en/stable/interactive/reference.html#manual-capture-of-command-output-and-magic-output
            >
            > E.g. found that quoting of curly brackets is done using .. etc.
            > If this documentation doesn't exist (I can't find where this is documented), please point to code
            > that handles this logic.
            >
            > The expansion of variables and expressions is done by the var_expand() method:
            > https://github.com/ipython/ipython/blob/8f50a3771614d3e22505eabc9332d1bc44af6f0e/IPython/core/interactiveshell.py#L3037
            >
            > Running a shell command and capturing the output is the getoutput() method:
            > https://github.com/ipython/ipython/blob/8f50a3771614d3e22505eabc9332d1bc44af6f0e/IPython/core/interactiveshell.py#L2279
            >
            > The escaped double curly braces ... is part of Python's standard string formatting machinery, which we use in var_expand().
            >

            githubbot ASF GitHub Bot added a comment - Github user Tagar commented on the issue: https://github.com/apache/zeppelin/pull/2502 Info from Jupyter comitter @takluyver > It's fairly brief, but here: > http://ipython.readthedocs.io/en/stable/interactive/reference.html#manual-capture-of-command-output-and-magic-output > > E.g. found that quoting of curly brackets is done using .. etc. > If this documentation doesn't exist (I can't find where this is documented), please point to code > that handles this logic. > > The expansion of variables and expressions is done by the var_expand() method: > https://github.com/ipython/ipython/blob/8f50a3771614d3e22505eabc9332d1bc44af6f0e/IPython/core/interactiveshell.py#L3037 > > Running a shell command and capturing the output is the getoutput() method: > https://github.com/ipython/ipython/blob/8f50a3771614d3e22505eabc9332d1bc44af6f0e/IPython/core/interactiveshell.py#L2279 > > The escaped double curly braces ... is part of Python's standard string formatting machinery, which we use in var_expand(). >
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            @felixcheung I have added a couple of paragraphs of documentation including an example showing interaction between the Scala and SQL interpreters to spark.md (at the end of the text under the title [Object Exchange](https://zeppelin.apache.org/docs/0.7.2/interpreter/spark.html#object-exchange) and just above the title [Form Creation](https://zeppelin.apache.org/docs/0.7.2/interpreter/spark.html#form-creation).

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 @felixcheung I have added a couple of paragraphs of documentation including an example showing interaction between the Scala and SQL interpreters to spark.md (at the end of the text under the title [Object Exchange] ( https://zeppelin.apache.org/docs/0.7.2/interpreter/spark.html#object-exchange ) and just above the title [Form Creation] ( https://zeppelin.apache.org/docs/0.7.2/interpreter/spark.html#form-creation ).
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Please let me know if there are any further comments or concerns @felixcheung. I have addressed the suggestion on structure of the code in the documentation ([here](https://github.com/apache/zeppelin/pull/2502/files/9b7673b7048992cbd07ccaad300abd16b4021521))

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Please let me know if there are any further comments or concerns @felixcheung. I have addressed the suggestion on structure of the code in the documentation ( [here] ( https://github.com/apache/zeppelin/pull/2502/files/9b7673b7048992cbd07ccaad300abd16b4021521 ))
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user Leemoonsoo commented on the issue:

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

            Testing this branch a bit and found there's an edge case with new line

            ```
            select
            *
            from

            {table}

            limit 10
            ````

            will throw error

            ```
            cannot recognize input near '

            {' 'table' '}

            ' in join source; line 3 pos 5
            set zeppelin.spark.sql.stacktrace = true to see full stacktrace
            ```

            githubbot ASF GitHub Bot added a comment - Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2502 Testing this branch a bit and found there's an edge case with new line ``` select * from {table} limit 10 ```` will throw error ``` cannot recognize input near ' {' 'table' '} ' in join source; line 3 pos 5 set zeppelin.spark.sql.stacktrace = true to see full stacktrace ```
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta closed the pull request at:

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

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta closed the pull request at: https://github.com/apache/zeppelin/pull/2502
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            GitHub user sanjaydasgupta reopened a pull request:

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

            ZEPPELIN-2807 Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967)

                1. What is this PR for?
                  This feature allows embedding/interpolating Z variables into SQL statements passed to the Spark-SQL interpreter. It implements one part of the functions desired by the pre-existing issue (ZEPPELIN-1967)
                1. What type of PR is it?
                  [Improvement]
                1. Todos
            • [ ] - Task
                1. What is the Jira issue?
                  https://issues.apache.org/jira/browse/ZEPPELIN-2807
                1. How should this be tested?
                  A new test has been added to the unit tests. The attached screenshot also shows tests of the functionality.
                1. Screenshots (if appropriate)
                  ![sparksqlchange](https://user-images.githubusercontent.com/477015/28521954-a1148532-7093-11e7-85e8-293d86fb0362.png)
                1. Questions:
            • Does the licenses files need update? No
            • Is there breaking changes for older versions? No
            • Does this needs documentation? Yes, a small update to any existing documentation is ideal.

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

            $ git pull https://github.com/sanjaydasgupta/zeppelin master

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

            https://github.com/apache/zeppelin/pull/2502.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 #2502


            commit e8ba58a94743b743f2a05cef8b9755d5bf79510c
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-24T11:29:29Z

            ZEPPELIN-2807: Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967)

            commit 565475d051db7bc30504233a0ca9293c413ca852
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-24T15:06:07Z

            Changed dist to precise in .travis.yml

            commit 2b3292a2ed03a50b1ca72400c3c1db277b52c7bf
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-24T15:15:36Z

            Changed dist to precise in .travis.yml - 2nd attempt

            commit 149eafa14f2831bfc656b126cb4ea55fbbf55cce
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-25T04:08:58Z

            Corrected regression caused by newly added unit test

            commit 47fde2fc6ab9f348d2702293c6ebe0fb389aa252
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-25T07:24:55Z

            Merge branch 'master' of git://github.com/apache/zeppelin

            commit f098ff111f48d7be04bbbbb6fd6176000f9f4bab
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-26T17:59:45Z

            Merge branch 'master' of git://github.com/apache/zeppelin

            commit 70fbbc1b5e07ac623dfa1ba873074082d6670562
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-29T04:23:56Z

            Added {{ for escaping single {, and more unit tests for all cases

            commit 901098a72560676888537398eb6d4412ca0b6e1f
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-29T05:11:53Z

            Aligned travis.yml with main apache/zeppelin repo

            commit 4e0d79e1637c2c9e7e9301ef81f1465b148d26dd
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-07-30T16:29:52Z

            Merge branch 'master' of git://github.com/apache/zeppelin

            commit 5e0ad610603e010648f89a7e987b713fe95e8705
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-04T16:37:01Z

            Merge branch 'master' of git://github.com/apache/zeppelin

            commit 28b095222f56772bbffaabed81968e44fb2a0380
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-10T15:40:18Z

            Merge branch 'master' of git://github.com/apache/zeppelin

            commit 3ca4ba1c2c16cebd884c2bc656932058b585886b
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-11T07:34:03Z

            Aligned travis.yml with main apache/zeppelin repo

            commit 861360343e8435b3cd9c3e7ec10a0ac382706f55
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-11T09:31:28Z

            Fixing errors in code-panel display

            commit 4f37495c6928044128140f0dd6942aff4500bf2b
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-11T12:57:51Z

            Merge branch 'master' of git://github.com/apache/zeppelin

            commit 9b7673b7048992cbd07ccaad300abd16b4021521
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-11T13:51:12Z

            Retrying with restored content

            commit d6cbd7205d6721ccff142b82d2e07626c90f8b94
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-12T14:23:18Z

            Changes to comply with review comments (https://github.com/apache/zeppelin/pull/2502/files/9b7673b7048992cbd07ccaad300abd16b4021521)

            commit 04acdd854436d5c4a505330b73e444c34e96ab9c
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-12T15:08:53Z

            Switched E2E tests to precise from trusty

            commit 906b698187bf5d4f358cb1263bdb15bfb0b286f4
            Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com>
            Date: 2017-08-12T15:14:35Z

            Revert "Switched E2E tests to precise from trusty" (commit 04acdd8)


            githubbot ASF GitHub Bot added a comment - GitHub user sanjaydasgupta reopened a pull request: https://github.com/apache/zeppelin/pull/2502 ZEPPELIN-2807 Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967 ) What is this PR for? This feature allows embedding/interpolating Z variables into SQL statements passed to the Spark-SQL interpreter. It implements one part of the functions desired by the pre-existing issue ( ZEPPELIN-1967 ) What type of PR is it? [Improvement] Todos [ ] - Task What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-2807 How should this be tested? A new test has been added to the unit tests. The attached screenshot also shows tests of the functionality. Screenshots (if appropriate) ! [sparksqlchange] ( https://user-images.githubusercontent.com/477015/28521954-a1148532-7093-11e7-85e8-293d86fb0362.png ) Questions: Does the licenses files need update? No Is there breaking changes for older versions? No Does this needs documentation? Yes, a small update to any existing documentation is ideal. You can merge this pull request into a Git repository by running: $ git pull https://github.com/sanjaydasgupta/zeppelin master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zeppelin/pull/2502.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 #2502 commit e8ba58a94743b743f2a05cef8b9755d5bf79510c Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-24T11:29:29Z ZEPPELIN-2807 : Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967 ) commit 565475d051db7bc30504233a0ca9293c413ca852 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-24T15:06:07Z Changed dist to precise in .travis.yml commit 2b3292a2ed03a50b1ca72400c3c1db277b52c7bf Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-24T15:15:36Z Changed dist to precise in .travis.yml - 2nd attempt commit 149eafa14f2831bfc656b126cb4ea55fbbf55cce Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-25T04:08:58Z Corrected regression caused by newly added unit test commit 47fde2fc6ab9f348d2702293c6ebe0fb389aa252 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-25T07:24:55Z Merge branch 'master' of git://github.com/apache/zeppelin commit f098ff111f48d7be04bbbbb6fd6176000f9f4bab Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-26T17:59:45Z Merge branch 'master' of git://github.com/apache/zeppelin commit 70fbbc1b5e07ac623dfa1ba873074082d6670562 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-29T04:23:56Z Added {{ for escaping single {, and more unit tests for all cases commit 901098a72560676888537398eb6d4412ca0b6e1f Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-29T05:11:53Z Aligned travis.yml with main apache/zeppelin repo commit 4e0d79e1637c2c9e7e9301ef81f1465b148d26dd Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-07-30T16:29:52Z Merge branch 'master' of git://github.com/apache/zeppelin commit 5e0ad610603e010648f89a7e987b713fe95e8705 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-04T16:37:01Z Merge branch 'master' of git://github.com/apache/zeppelin commit 28b095222f56772bbffaabed81968e44fb2a0380 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-10T15:40:18Z Merge branch 'master' of git://github.com/apache/zeppelin commit 3ca4ba1c2c16cebd884c2bc656932058b585886b Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-11T07:34:03Z Aligned travis.yml with main apache/zeppelin repo commit 861360343e8435b3cd9c3e7ec10a0ac382706f55 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-11T09:31:28Z Fixing errors in code-panel display commit 4f37495c6928044128140f0dd6942aff4500bf2b Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-11T12:57:51Z Merge branch 'master' of git://github.com/apache/zeppelin commit 9b7673b7048992cbd07ccaad300abd16b4021521 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-11T13:51:12Z Retrying with restored content commit d6cbd7205d6721ccff142b82d2e07626c90f8b94 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-12T14:23:18Z Changes to comply with review comments ( https://github.com/apache/zeppelin/pull/2502/files/9b7673b7048992cbd07ccaad300abd16b4021521 ) commit 04acdd854436d5c4a505330b73e444c34e96ab9c Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-12T15:08:53Z Switched E2E tests to precise from trusty commit 906b698187bf5d4f358cb1263bdb15bfb0b286f4 Author: Sanjay Dasgupta <sanjay.dasgupta@gmail.com> Date: 2017-08-12T15:14:35Z Revert "Switched E2E tests to precise from trusty" (commit 04acdd8)
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Thanks for the catch # @Leemoonsoo

            I have fixed and committed the issue.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Thanks for the catch # @Leemoonsoo I have fixed and committed the issue.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user Leemoonsoo commented on the issue:

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

            @sanjaydasgupta Thanks for addressing newline issue.

            If we take a look dynamic form, each interpreter can choose whether interpreter wants to use Dynamic form or not by overriding [Interpreter. getFormType()](https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java#L102).

            Do you think this feature can be generalized and each interpreter will able to choose to enable/disable this feature, in the future?

            githubbot ASF GitHub Bot added a comment - Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2502 @sanjaydasgupta Thanks for addressing newline issue. If we take a look dynamic form, each interpreter can choose whether interpreter wants to use Dynamic form or not by overriding [Interpreter. getFormType()] ( https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java#L102 ). Do you think this feature can be generalized and each interpreter will able to choose to enable/disable this feature, in the future?
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            I had not thought about that @Leemoonsoo, thanks for alerting me to it.

            As the code now stands (in relation to [Interpreter.java](https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java) and it's concrete child classes) object interpolation is not automatically applied to the command string when [interpret(...)](https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java#L86) is called. So, it is as if the feature is off by default, and each interpreter can opt in by adding a few lines of code to its overridden version of `interpret(...)` for calling the command interpolating function [SparkSqlInterpreter.interpolateVariable(...)](https://github.com/sanjaydasgupta/zeppelin/blob/906b698187bf5d4f358cb1263bdb15bfb0b286f4/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java#L92).

            In the long run (when more interpreters use this feature), the interpolating function [SparkSqlInterpreter.interpolateVariable(...)](https://github.com/sanjaydasgupta/zeppelin/blob/906b698187bf5d4f358cb1263bdb15bfb0b286f4/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java#L92) could be moved up into `Interpreter.java`, and the command interpolation logic could be applied automatically – conditionally upon the return value of a function like [Interpreter. getFormType()](https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java#L102).

            My feeling is that fairly extensive refactoring affecting all the existing interpreter classes will be needed when the changes in the previous paragraph are made. But I can take that path if you think it is preferable.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 I had not thought about that @Leemoonsoo, thanks for alerting me to it. As the code now stands (in relation to [Interpreter.java] ( https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java ) and it's concrete child classes) object interpolation is not automatically applied to the command string when [interpret(...)] ( https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java#L86 ) is called. So, it is as if the feature is off by default, and each interpreter can opt in by adding a few lines of code to its overridden version of `interpret(...)` for calling the command interpolating function [SparkSqlInterpreter.interpolateVariable(...)] ( https://github.com/sanjaydasgupta/zeppelin/blob/906b698187bf5d4f358cb1263bdb15bfb0b286f4/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java#L92 ). In the long run (when more interpreters use this feature), the interpolating function [SparkSqlInterpreter.interpolateVariable(...)] ( https://github.com/sanjaydasgupta/zeppelin/blob/906b698187bf5d4f358cb1263bdb15bfb0b286f4/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java#L92 ) could be moved up into `Interpreter.java`, and the command interpolation logic could be applied automatically – conditionally upon the return value of a function like [Interpreter. getFormType()] ( https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java#L102 ). My feeling is that fairly extensive refactoring affecting all the existing interpreter classes will be needed when the changes in the previous paragraph are made. But I can take that path if you think it is preferable.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user Leemoonsoo commented on the issue:

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

            Thanks, @sanjaydasgupta for sharing your thoughts.
            I just wanted to think about possibility generalization of this feature for the future.

            LGTM

            githubbot ASF GitHub Bot added a comment - Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2502 Thanks, @sanjaydasgupta for sharing your thoughts. I just wanted to think about possibility generalization of this feature for the future. LGTM
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Allow me to take your suggestions from last to first @felixcheung

            I have tested a variant of the function [interpolateVariable(...)](https://github.com/apache/zeppelin/pull/2502/files/906b698187bf5d4f358cb1263bdb15bfb0b286f4#diff-851965d16af3e64741e7375e5bf9b6c5R92) to return a `scala.Tuple2[Bool, String]` (with the Bool indicating success/failure, and the String containing either the interpolated command or the error message). The resulting code is elegant and intuitive, but could be a stumbling block for those not familiar with Scala. Please let me know if it is ok to add a bit of Scala in this way.

            After thinking hard about the implications (to other interpreters) of assumptions we make now about the legal forms of the use of '

            {' and '}', I tried to seek inspiration from Jupyter's implementation of a similar feature (when spawning a shell command using !). The following clear cases exist:

            1) any `{varname}` (formed with exactly one '{' and '}

            ') is interpolated into the command – only if `varname` is defined. If `varname` is not defined, no error is reported, and the pattern is left as is – to be handled by the spawned shell command.

            2) any `something` (formed with exactly two '

            {' and '}' characters) is interpolated as `{something}`. There is no change in this behavior even when `something` has a defined value.

            3) All other occurrences of '{' and '}

            ' (singly or in groups) are treated as ordinary characters, and passed through unchanged to the underlying shell command.

            Considering all the different usages of '

            {' and '}

            ' that there might be in many different interpreters (to which we extend this feature progressively), it may be reasonable to follow something like the above convention from Jupyter.

            Please let me know your views on these. Thanks for your time and help with this.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Allow me to take your suggestions from last to first @felixcheung I have tested a variant of the function [interpolateVariable(...)] ( https://github.com/apache/zeppelin/pull/2502/files/906b698187bf5d4f358cb1263bdb15bfb0b286f4#diff-851965d16af3e64741e7375e5bf9b6c5R92 ) to return a `scala.Tuple2 [Bool, String] ` (with the Bool indicating success/failure, and the String containing either the interpolated command or the error message). The resulting code is elegant and intuitive, but could be a stumbling block for those not familiar with Scala. Please let me know if it is ok to add a bit of Scala in this way. After thinking hard about the implications (to other interpreters) of assumptions we make now about the legal forms of the use of ' {' and '}', I tried to seek inspiration from Jupyter's implementation of a similar feature (when spawning a shell command using !). The following clear cases exist: 1) any `{varname}` (formed with exactly one '{' and '} ') is interpolated into the command – only if `varname` is defined. If `varname` is not defined, no error is reported, and the pattern is left as is – to be handled by the spawned shell command. 2) any ` something ` (formed with exactly two ' {' and '}' characters) is interpolated as `{something}`. There is no change in this behavior even when `something` has a defined value. 3) All other occurrences of '{' and '} ' (singly or in groups) are treated as ordinary characters, and passed through unchanged to the underlying shell command. Considering all the different usages of ' {' and '} ' that there might be in many different interpreters (to which we extend this feature progressively), it may be reasonable to follow something like the above convention from Jupyter. Please let me know your views on these. Thanks for your time and help with this.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Thanks for the suggestions @felixcheung

            I was not able to find more documentation on Jupyter's feature, but tried it with many different variants. There are some behaviors that had not been found earlier, so I have added it to the implementation and the documentation. I also gave the documentation section a more distinctive title.

            One of the things I learned about Jupyter's implementation is that it never intercepts an error case. All command strings are passed through to the underlying command – with or without interpolation. Any use of '

            {' and/or '}

            ' that does not appear to be part of a well-formed pattern causes Jupyter to pass on the entire command string unchanged. This happens even when the command string contains other patterns that are well-formed, and could be interpolated.

            So, there is now no need to return a status value to indicate the success/failure of the interpolation. The string returned by the interpolation function is generated by emulating Jupyter's behavior in the same situation.

            I have also moved the interpolation function into the superclass [Interpreter.java](https://github.com/sanjaydasgupta/zeppelin/commit/9e75333ff7ef9c27678382c598b524eafc6191f7#diff-f9d22f7302ae356454cdcc637942856fR493) to enable it to be shared and used by other interpreters too. This follows a suggestion from @Leemoonsoo earlier in the PR. My implementation of this feature for the Shell interpreter is also ready, and I will submit a PR once this one is done.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Thanks for the suggestions @felixcheung I was not able to find more documentation on Jupyter's feature, but tried it with many different variants. There are some behaviors that had not been found earlier, so I have added it to the implementation and the documentation. I also gave the documentation section a more distinctive title. One of the things I learned about Jupyter's implementation is that it never intercepts an error case. All command strings are passed through to the underlying command – with or without interpolation. Any use of ' {' and/or '} ' that does not appear to be part of a well-formed pattern causes Jupyter to pass on the entire command string unchanged. This happens even when the command string contains other patterns that are well-formed, and could be interpolated. So, there is now no need to return a status value to indicate the success/failure of the interpolation. The string returned by the interpolation function is generated by emulating Jupyter's behavior in the same situation. I have also moved the interpolation function into the superclass [Interpreter.java] ( https://github.com/sanjaydasgupta/zeppelin/commit/9e75333ff7ef9c27678382c598b524eafc6191f7#diff-f9d22f7302ae356454cdcc637942856fR493 ) to enable it to be shared and used by other interpreters too. This follows a suggestion from @Leemoonsoo earlier in the PR. My implementation of this feature for the Shell interpreter is also ready, and I will submit a PR once this one is done.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user Tagar commented on the issue:

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

            My two cents

            > One of the things I learned about Jupyter's implementation is that it never intercepts an error case

            I personally not necessarily like how this piece is implemented in Jupyter.. that an error can be silently ignored, and in case of an error whole

            {.. }

            block is returned unchanged.

            Should we have a Zeppelin option that controls this behavior?
            1. Option 1 if used - it'll completely follow Jupyter bahavior (no error,

            {..}

            block return unchanged).
            2. Option 2 - fail explicitly if variable in curly braces can't be found.

            That being said, I think most of folks may actually prefer option 1, as introduction of

            { .. }

            may be a breaking change for some notebooks' code. So I think Option 1 should be default.

            > This follows a suggestion from @Leemoonsoo earlier in the PR. My implementation of this feature for the Shell interpreter is also ready, and I will submit a PR once this one is done.

            That's great. Thanks for working on this. I see there are so many good use cases for this feature.

            githubbot ASF GitHub Bot added a comment - Github user Tagar commented on the issue: https://github.com/apache/zeppelin/pull/2502 My two cents > One of the things I learned about Jupyter's implementation is that it never intercepts an error case I personally not necessarily like how this piece is implemented in Jupyter.. that an error can be silently ignored, and in case of an error whole {.. } block is returned unchanged. Should we have a Zeppelin option that controls this behavior? 1. Option 1 if used - it'll completely follow Jupyter bahavior (no error, {..} block return unchanged). 2. Option 2 - fail explicitly if variable in curly braces can't be found. That being said, I think most of folks may actually prefer option 1, as introduction of { .. } may be a breaking change for some notebooks' code. So I think Option 1 should be default. > This follows a suggestion from @Leemoonsoo earlier in the PR. My implementation of this feature for the Shell interpreter is also ready, and I will submit a PR once this one is done. That's great. Thanks for working on this. I see there are so many good use cases for this feature.
            coach Oletix Coach added a comment -

            I am trying to pass collection vars to %sql, but it doesn't work. The workaround that worked for me was to use z.angularBind() and then use ${varname} in %sql. Hope this feature is available soon.

            coach Oletix Coach added a comment - I am trying to pass collection vars to %sql, but it doesn't work. The workaround that worked for me was to use z.angularBind() and then use ${varname} in %sql. Hope this feature is available soon.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            Would like to hear about any further improvements required on this feature @felixcheung, @Leemoonsoo

            Thanks for your thoughts @Tagar

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 Would like to hear about any further improvements required on this feature @felixcheung, @Leemoonsoo Thanks for your thoughts @Tagar
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user felixcheung commented on the issue:

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

            could you review my comments? there are a number of comments from before that I feel we should address first.

            githubbot ASF GitHub Bot added a comment - Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2502 could you review my comments? there are a number of comments from before that I feel we should address first.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta commented on the issue:

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

            @felixcheung - We've exchanged many ideas back and forth on this PR to the point where it has become more of a discussion document than a software submission

            I'm in favor of closing this PR, and starting over with a new PR that gathers together all of the learnings from the many suggestions and review comments in this one. That should also allow me to fix somethings that seem to have got messed with my repo – I'm relatively new to git, and am not being able to reverse some of this damage.

            Let me know if you agree, or if there is a better way to take this forward.

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2502 @felixcheung - We've exchanged many ideas back and forth on this PR to the point where it has become more of a discussion document than a software submission I'm in favor of closing this PR, and starting over with a new PR that gathers together all of the learnings from the many suggestions and review comments in this one. That should also allow me to fix somethings that seem to have got messed with my repo – I'm relatively new to git, and am not being able to reverse some of this damage. Let me know if you agree, or if there is a better way to take this forward.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user felixcheung commented on the issue:

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

            Sure - feel free to close this and open a new PR.

            githubbot ASF GitHub Bot added a comment - Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2502 Sure - feel free to close this and open a new PR.
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ] This issue links to "GitHub Pull Request #2502 (Web Link)" [ 86826 ]
            githubbot ASF GitHub Bot added a comment -

            Github user sanjaydasgupta closed the pull request at:

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

            githubbot ASF GitHub Bot added a comment - Github user sanjaydasgupta closed the pull request at: https://github.com/apache/zeppelin/pull/2502
            sanjay.dasgupta@gmail.com Sanjay Dasgupta added a comment - - edited

            This issue is not needed any more as the latest implementation path (see PR-2834) directly references the parent issue (ZEPPELIN-1967)

            sanjay.dasgupta@gmail.com Sanjay Dasgupta added a comment - - edited This issue is not needed any more as the latest implementation path (see PR-2834 ) directly references the parent issue ( ZEPPELIN-1967 )
            sanjay.dasgupta@gmail.com Sanjay Dasgupta made changes -
            Resolution Won't Do [ 10011 ]
            Status Open [ 1 ] Closed [ 6 ]

            People

              Unassigned Unassigned
              sanjay.dasgupta@gmail.com Sanjay Dasgupta
              Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: