Uploaded image for project: 'Apache NiFi'
  1. Apache NiFi
  2. NIFI-5083

Support for Expression Language in Group ID field of Kafka Consumer Processors

Details

    • Improvement
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • 1.6.0
    • 1.8.0
    • Extensions

    Description

      I have the use case where I have two separate NiFi clusters reading data from the same topics but need to have unique group ids so they each get separate copies of the data.  It would be useful if the group id field supported expression language so I could use a processor group variable to define the group id to use.  I have a large enough number of kafka consumers to make this tedious to manually change.

      Attachments

        1. NIFI-5083.mp4
          14.76 MB
          Corey Fritz

        Issue Links

          Activity

            joewitt Joe Witt added a comment -

            rbruno does this mean you're using the FlowRegistry to ensure both clusters have a consistent flow and thus are ending up with the same group id being used?

            Adding support for EL to let variables be used is a very doable option and a good idea.  Just wanted to understand how you're keeping the flows so in sync between clusters that this would be matching at this time.

             

            Thanks

            joewitt Joe Witt added a comment - rbruno does this mean you're using the FlowRegistry to ensure both clusters have a consistent flow and thus are ending up with the same group id being used? Adding support for EL to let variables be used is a very doable option and a good idea.  Just wanted to understand how you're keeping the flows so in sync between clusters that this would be matching at this time.   Thanks
            rbruno Robert Bruno added a comment -

            Currently I am using templates to sync between instances.  I will likely for the time being do a find and replace in the template file, but this seems far from ideal.  

             

            With that said FlowRegistry is on my short list of new things to give a try. Hopefully it is a better way to keep my 2 clusters in sync along with potentially dev clusters.

            rbruno Robert Bruno added a comment - Currently I am using templates to sync between instances.  I will likely for the time being do a find and replace in the template file, but this seems far from ideal.     With that said FlowRegistry is on my short list of new things to give a try. Hopefully it is a better way to keep my 2 clusters in sync along with potentially dev clusters.
            snagafritz Corey Fritz added a comment -

            I would also like to be able to use EL to set consumer group ids. I'm going to take a crack at this.

            snagafritz Corey Fritz added a comment - I would also like to be able to use EL to set consumer group ids. I'm going to take a crack at this.
            githubbot ASF GitHub Bot added a comment -

            GitHub user snagafritz opened a pull request:

            https://github.com/apache/nifi/pull/2913

            NIFI-5083: enabled EL support for processor configuration of Kafka consumer group id

            Thank you for submitting a contribution to Apache NiFi.

            In order to streamline the review of the contribution we ask you
            to ensure the following steps have been taken:

                1. For all changes:
            • [x] Is there a JIRA ticket associated with this PR? Is it referenced
              in the commit message?
            • [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
            • [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
            • [x] Is your initial contribution a single, squashed commit?
                1. For code changes:
            • [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
            • [ ] Have you written or updated unit tests to verify your changes?

            ```
            I started to write unit tests for these changes then realized that the tests were really verifying the evaluation of the EL expressions, and I assume far better unit tests for this already exist. I will attach a video of me verifying expected behavior of a build of this branch to the JIRA ticket.
            ```

            • [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
            • [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
            • [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
            • [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
                1. For documentation related changes:
            • [x] Have you ensured that format looks appropriate for the output in which it is rendered?
                1. Note:
                  Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

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

            $ git pull https://github.com/Snagajob/nifi NIFI-5083

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

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


            commit 9d0a1ecd1d83ee2c6e54e3018db41280876a9629
            Author: Corey Fritz <corey.fritz@...>
            Date: 2018-07-07T05:28:50Z

            NIFI-5083: enabled EL support for processor configuration of Kafka consumer `group.id` property


            githubbot ASF GitHub Bot added a comment - GitHub user snagafritz opened a pull request: https://github.com/apache/nifi/pull/2913 NIFI-5083 : enabled EL support for processor configuration of Kafka consumer group id Thank you for submitting a contribution to Apache NiFi. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: For all changes: [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. [x] Has your PR been rebased against the latest commit within the target branch (typically master)? [x] Is your initial contribution a single, squashed commit? For code changes: [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder? [ ] Have you written or updated unit tests to verify your changes? ``` I started to write unit tests for these changes then realized that the tests were really verifying the evaluation of the EL expressions, and I assume far better unit tests for this already exist. I will attach a video of me verifying expected behavior of a build of this branch to the JIRA ticket. ``` [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0] ( http://www.apache.org/legal/resolved.html#category-a)? [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly? [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly? [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties? For documentation related changes: [x] Have you ensured that format looks appropriate for the output in which it is rendered? Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/Snagajob/nifi NIFI-5083 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/2913.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 #2913 commit 9d0a1ecd1d83ee2c6e54e3018db41280876a9629 Author: Corey Fritz <corey.fritz@...> Date: 2018-07-07T05:28:50Z NIFI-5083 : enabled EL support for processor configuration of Kafka consumer `group.id` property
            snagafritz Corey Fritz added a comment -

            Attached video of me verifying expected behavior from a build of my branch.

            snagafritz Corey Fritz added a comment - Attached video of me verifying expected behavior from a build of my branch.
            githubbot ASF GitHub Bot added a comment -

            Github user bbende commented on the issue:

            https://github.com/apache/nifi/pull/2913

            I think you will also need to find where the group id property is used and ensure that evaluateAttributeExpressions() is called when obtaining the value.

            githubbot ASF GitHub Bot added a comment - Github user bbende commented on the issue: https://github.com/apache/nifi/pull/2913 I think you will also need to find where the group id property is used and ensure that evaluateAttributeExpressions() is called when obtaining the value.
            githubbot ASF GitHub Bot added a comment -

            Github user bbende commented on the issue:

            https://github.com/apache/nifi/pull/2913

            Actually, I take back my last comment, I now see where the value is obtained and it does have the logic:
            ```
            String propertyValue = propertyDescriptor.isExpressionLanguageSupported()
            ? context.getProperty(propertyDescriptor).evaluateAttributeExpressions().getValue()
            : context.getProperty(propertyDescriptor).getValue();
            ```
            Sorry about that.

            githubbot ASF GitHub Bot added a comment - Github user bbende commented on the issue: https://github.com/apache/nifi/pull/2913 Actually, I take back my last comment, I now see where the value is obtained and it does have the logic: ``` String propertyValue = propertyDescriptor.isExpressionLanguageSupported() ? context.getProperty(propertyDescriptor).evaluateAttributeExpressions().getValue() : context.getProperty(propertyDescriptor).getValue(); ``` Sorry about that.
            githubbot ASF GitHub Bot added a comment -

            Github user zenfenan commented on the issue:

            https://github.com/apache/nifi/pull/2913

            Reviewing..

            githubbot ASF GitHub Bot added a comment - Github user zenfenan commented on the issue: https://github.com/apache/nifi/pull/2913 Reviewing..

            Commit 4baf35db5ba296d246e3491e8d8c84408306f685 in nifi's branch refs/heads/master from snagafritz
            [ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=4baf35d ]

            NIFI-5083: enabled EL support for processor configuration of Kafka consumer `group.id` property

            Signed-off-by: zenfenan <zenfenan@apache.org>

            jira-bot ASF subversion and git services added a comment - Commit 4baf35db5ba296d246e3491e8d8c84408306f685 in nifi's branch refs/heads/master from snagafritz [ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=4baf35d ] NIFI-5083 : enabled EL support for processor configuration of Kafka consumer `group.id` property Signed-off-by: zenfenan <zenfenan@apache.org>
            githubbot ASF GitHub Bot added a comment -

            Github user zenfenan commented on the issue:

            https://github.com/apache/nifi/pull/2913

            +1 LGTM. Merged to master. Thanks @snagafritz

            githubbot ASF GitHub Bot added a comment - Github user zenfenan commented on the issue: https://github.com/apache/nifi/pull/2913 +1 LGTM. Merged to master. Thanks @snagafritz
            githubbot ASF GitHub Bot added a comment -

            Github user zenfenan commented on the issue:

            https://github.com/apache/nifi/pull/2913

            @snagafritz Hey.. I forgot to close this PR while merging. It is now merged to the master. Can you manually close this PR?

            githubbot ASF GitHub Bot added a comment - Github user zenfenan commented on the issue: https://github.com/apache/nifi/pull/2913 @snagafritz Hey.. I forgot to close this PR while merging. It is now merged to the master. Can you manually close this PR?
            githubbot ASF GitHub Bot added a comment -

            Github user snagafritz closed the pull request at:

            https://github.com/apache/nifi/pull/2913

            githubbot ASF GitHub Bot added a comment - Github user snagafritz closed the pull request at: https://github.com/apache/nifi/pull/2913

            People

              Unassigned Unassigned
              rbruno Robert Bruno
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: