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

Posted to issues@nifi.apache.org by david-streamlio <gi...@git.apache.org> on 2018/02/26 16:06:03 UTC

[GitHub] nifi pull request #2493: Added Pulsar processors and Controller Service

GitHub user david-streamlio opened a pull request:

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

    Added Pulsar processors and Controller Service

    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:
    - [ ] Is there a JIRA ticket associated with this PR? Is it referenced 
         in the commit message?
    
    - [ ] 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.
    
    - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [ ] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [ ] 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?
    - [ ] 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:
    - [ ] 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/david-streamlio/nifi NIFI-4908

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

    https://github.com/apache/nifi/pull/2493.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 #2493
    
----
commit e4f8550159132d85d0e19d61f24e087e51742dee
Author: David Kjerrumgaard <da...@...>
Date:   2018-02-24T01:40:22Z

    Added Pulsar processors and Controller Service

----


---

[GitHub] nifi issue #2493: Added Pulsar processors and Controller Service

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

Github user joewitt commented on the issue:

    https://github.com/apache/nifi/pull/2493
  
    please update all the nifi component version references to 1.6.0-SNAPSHOT


---

[GitHub] nifi issue #2493: Added Pulsar processors and Controller Service

Posted by david-streamlio <gi...@git.apache.org>.

Github user david-streamlio commented on the issue:

    https://github.com/apache/nifi/pull/2493
  
    When I started writing these processors, I used the Kafka ones as a model. However, I felt that it would be easier to configure the Pulsar client once, such as SSLContextService integration, and then pool both the Producer and Consumers for re-use, similar to a connection pool for a database.


---

[GitHub] nifi issue #2493: Added Pulsar processors and Controller Service

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

Github user tasanuma commented on the issue:

    https://github.com/apache/nifi/pull/2493
  
    Hi @david-streamlio. Thanks for the work. This would really helpful for Pulsar users.
    
    I have one question. Are there any reasons that this processor uses Controller Service? I think this is similar to Kafka processors, but they don't use Controller Service.


---

[GitHub] nifi issue #2493: Added Pulsar processors and Controller Service

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

Github user markap14 commented on the issue:

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

Hey @david-streamlio this is very cool! I've been thinking about writing processors for interacting with Pulsar myself but haven't had a chance yet. Just a few things that we should think through a bit:

- Re: Connection Pool in controller service vs. doing it in the processor: what makes sense here I think depends on how you expect to use it. If you expect to be creating several Pulsar processors with the same connection info, then a Controller Service makes sense. If you think the more common case will be a single instance of the Processor then configuring it in the Processor is probably easier for the user. I think both have their merits though, so I'm fine with either approach personally.

- One concern that I have is that with the Kafka processors, we end up having to create a new copy of the processors with pretty much each release of Kafka, so that we can take advantage of the new features. Have you considered how you see this evolving as more versions of Pulsar are released? There are two approaches that we often see with NiFi. One is to create a new processor for each new version as we did with Kafka. The other is to have a "Client Service" Controller service. It would then have methods like publish(FlowFile), consume() or something like that. Then there is only a single ConsumePulsar processor and a single PublishPulsar processor. Each is then just configured with the controller service that handles interacting with Pulsar directly. Either approach is okay, I think. But we should probably think about naming at least - does it make sense to name these ConsumePulsar_1_20 or ConsumePulsar_1_0 or something of that nature? I think it's best to figure this part out
before the initial release because it can then become confusing if we have processors like ConsumePulsar and ConsumePulsar_1_35 for instance.

Thoughts?

---

[GitHub] nifi issue #2493: Added Pulsar processors and Controller Service

Posted by david-streamlio <gi...@git.apache.org>.

Github user david-streamlio commented on the issue:

    https://github.com/apache/nifi/pull/2493
  
    I have rebased my branch and created new pull request here:  https://github.com/apache/nifi/pull/2553. Please ignore this pull request as the new one supersedes it.


---