You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by olegz <gi...@git.apache.org> on 2016/09/20 16:22:32 UTC

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

GitHub user olegz opened a pull request:

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

    NIFI-2774 added configurable QoS options

    

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

    $ git pull https://github.com/olegz/nifi NIFI-2774B

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

    https://github.com/apache/nifi/pull/1036.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 #1036
    
----
commit 5df326d886cb806d345468cd08ecd34f5b85cdd8
Author: Oleg Zhurakousky <ol...@suitcase.io>
Date:   2016-09-20T14:49:22Z

    NIFI-2774 added configurable QoS options

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by mosermw <gi...@git.apache.org>.
Github user mosermw commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1036#discussion_r82003040
  
    --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
    @@ -54,8 +61,31 @@
     @SeeAlso(value = { PublishJMS.class, JMSConnectionFactoryProvider.class })
     public class ConsumeJMS extends AbstractJMSProcessor<JMSConsumer> {
     
    +    static final AllowableValue AUTO_ACK = new AllowableValue(String.valueOf(Session.AUTO_ACKNOWLEDGE),
    --- End diff --
    
    @olegz would it be worthwhile to change the AllowableValue displayNames to "Auto Acknowledge (1)" and "Client Acknowledge (2)" (for example) or am I just overthinking this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1036#discussion_r81854487
  
    --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
    @@ -54,8 +61,31 @@
     @SeeAlso(value = { PublishJMS.class, JMSConnectionFactoryProvider.class })
     public class ConsumeJMS extends AbstractJMSProcessor<JMSConsumer> {
     
    +    static final AllowableValue AUTO_ACK = new AllowableValue(String.valueOf(Session.AUTO_ACKNOWLEDGE),
    --- End diff --
    
    I see what you're saying, but it's out of my control since the actual values is defined by JMS Session interface and 1 is auto-ack and 2 is client-ack which is what this JIRA is all about.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #1036: NIFI-2774 added configurable QoS options

Posted by mosermw <gi...@git.apache.org>.
Github user mosermw commented on the issue:

    https://github.com/apache/nifi/pull/1036
  
    +1 Looks great to me.  I won't have time to merge for the next few hours, so if you want to do so then please do.  Then let @trkurc know this PR is complete and ready for 0.x release.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1036#discussion_r82003485
  
    --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
    @@ -54,8 +61,31 @@
     @SeeAlso(value = { PublishJMS.class, JMSConnectionFactoryProvider.class })
     public class ConsumeJMS extends AbstractJMSProcessor<JMSConsumer> {
     
    +    static final AllowableValue AUTO_ACK = new AllowableValue(String.valueOf(Session.AUTO_ACKNOWLEDGE),
    --- End diff --
    
    Hmm, didn't think of that, but "I like!" ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #1036: NIFI-2774 added configurable QoS options

Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the issue:

    https://github.com/apache/nifi/pull/1036
  
    @mosermw all PR comments are addressed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by mosermw <gi...@git.apache.org>.
Github user mosermw commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1036#discussion_r81410549
  
    --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
    @@ -54,8 +61,31 @@
     @SeeAlso(value = { PublishJMS.class, JMSConnectionFactoryProvider.class })
     public class ConsumeJMS extends AbstractJMSProcessor<JMSConsumer> {
     
    +    static final AllowableValue AUTO_ACK = new AllowableValue(String.valueOf(Session.AUTO_ACKNOWLEDGE),
    +            "AUTO_ACKNOWLEDGE",
    +            "Automatically acknowledges a client's receipt of a message, regardless if NiFi session has been commited. "
    +                    + "Can result in data loss in the event where NiFi abruptly stopped before session was commited.");
    +
    +    static final AllowableValue CLIENT_ACK = new AllowableValue(String.valueOf(Session.CLIENT_ACKNOWLEDGE),
    +            "CLIENT_ACKNOWLEDGE",
    +            "(DEFAULT) Manually acknowledges a client's receipt of a message after NiFi Session was commited, thus ensuring no data loss");
    +
    +    static final AllowableValue DUPS_OK = new AllowableValue(String.valueOf(Session.DUPS_OK_ACKNOWLEDGE),
    +            "DUPS_OK_ACKNOWLEDGE",
    +            "This acknowledgment mode instructs the session to lazily acknowledge the delivery of messages. May result uin both data "
    --- End diff --
    
    Spelling "uin"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by Jeff <jt...@gmail.com>.
The Apache NiFi mailing lists are documented here, with links to
subscribe/unsubscribe: https://nifi.apache.org/mailing_lists.html

It looks like you want to unsubscribe from the Issues mailing list; sending
an email to issues-unsubscribe@nifi.apache.org will unsubscribe you.

-- Jeff

On Tue, Oct 4, 2016 at 5:25 PM Maykiwo GNO <ma...@gmail.com> wrote:

> Hi all,
>
> Sorry but how unsubscribe from this mailing list?
>
> Regards
>
> Le 4 oct. 2016 23:07, "mosermw" <gi...@git.apache.org> a écrit :
>
> > Github user mosermw commented on a diff in the pull request:
> >
> >     https://github.com/apache/nifi/pull/1036#discussion_r81854062
> >
> >     --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/
> > src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
> >     @@ -54,8 +61,31 @@
> >      @SeeAlso(value = { PublishJMS.class,
> JMSConnectionFactoryProvider.class
> > })
> >      public class ConsumeJMS extends AbstractJMSProcessor<JMSConsumer> {
> >
> >     +    static final AllowableValue AUTO_ACK = new
> > AllowableValue(String.valueOf(Session.AUTO_ACKNOWLEDGE),
> >     --- End diff --
> >
> >     It's a bit confusing to see "Default value: 2" when I hover over the
> > tooltip for the Acknowledgement Mode property.  It looks like the
> > serialized values will be "1", "2", or "3".  Perhaps this is more of a
> > problem in the UI rather than AllowableValue definition.
> >
> >
> > ---
> > If your project is set up for it, you can reply to this email and have
> your
> > reply appear on GitHub as well. If your project does not have this
> feature
> > enabled and wishes so, or if the feature is enabled but not working,
> please
> > contact infrastructure at infrastructure@apache.org or file a JIRA
> ticket
> > with INFRA.
> > ---
> >
>

Re: [GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by Maykiwo GNO <ma...@gmail.com>.
Hi all,

Sorry but how unsubscribe from this mailing list?

Regards

Le 4 oct. 2016 23:07, "mosermw" <gi...@git.apache.org> a écrit :

> Github user mosermw commented on a diff in the pull request:
>
>     https://github.com/apache/nifi/pull/1036#discussion_r81854062
>
>     --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/
> src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
>     @@ -54,8 +61,31 @@
>      @SeeAlso(value = { PublishJMS.class, JMSConnectionFactoryProvider.class
> })
>      public class ConsumeJMS extends AbstractJMSProcessor<JMSConsumer> {
>
>     +    static final AllowableValue AUTO_ACK = new
> AllowableValue(String.valueOf(Session.AUTO_ACKNOWLEDGE),
>     --- End diff --
>
>     It's a bit confusing to see "Default value: 2" when I hover over the
> tooltip for the Acknowledgement Mode property.  It looks like the
> serialized values will be "1", "2", or "3".  Perhaps this is more of a
> problem in the UI rather than AllowableValue definition.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by mosermw <gi...@git.apache.org>.
Github user mosermw commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1036#discussion_r81854062
  
    --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
    @@ -54,8 +61,31 @@
     @SeeAlso(value = { PublishJMS.class, JMSConnectionFactoryProvider.class })
     public class ConsumeJMS extends AbstractJMSProcessor<JMSConsumer> {
     
    +    static final AllowableValue AUTO_ACK = new AllowableValue(String.valueOf(Session.AUTO_ACKNOWLEDGE),
    --- End diff --
    
    It's a bit confusing to see "Default value: 2" when I hover over the tooltip for the Acknowledgement Mode property.  It looks like the serialized values will be "1", "2", or "3".  Perhaps this is more of a problem in the UI rather than AllowableValue definition.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1036#discussion_r81854663
  
    --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
    @@ -116,18 +154,26 @@ protected JMSConsumer finishBuildingTargetResource(JmsTemplate jmsTemplate) {
         }
     
         /**
    +     *
    +     */
    +    @Override
    +    protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
    +        List<PropertyDescriptor> pd = new ArrayList<>(super.getSupportedPropertyDescriptors());
    +        pd.add(ACKNOWLEDGEMENT_MODE);
    +        return pd;
    --- End diff --
    
    Fair point. will fix


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1036: NIFI-2774 added configurable QoS options

Posted by mosermw <gi...@git.apache.org>.
Github user mosermw commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1036#discussion_r81411506
  
    --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java ---
    @@ -116,18 +154,26 @@ protected JMSConsumer finishBuildingTargetResource(JmsTemplate jmsTemplate) {
         }
     
         /**
    +     *
    +     */
    +    @Override
    +    protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
    +        List<PropertyDescriptor> pd = new ArrayList<>(super.getSupportedPropertyDescriptors());
    +        pd.add(ACKNOWLEDGEMENT_MODE);
    +        return pd;
    --- End diff --
    
    Not an incredibly big deal, but getSupportedPropertyDescriptors() is called very often (most often in validation during a UI Refresh) which causes lots of ArrayLists to be created.  I prefer the way DistributeLoad handles this, by creating one ArrayList and reusing it, while also using an AtomicBoolean to know when it should be recreated.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---