You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by JPercivall <gi...@git.apache.org> on 2016/04/01 23:45:56 UTC

[GitHub] nifi pull request: NIFI-1582 added state to UpdateAttribute as wel...

GitHub user JPercivall opened a pull request:

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

    NIFI-1582 added state to UpdateAttribute as well as updated a few par…

    …ts that hadn't be touched in years (referenced the 'FlowFileMetadataEnhancer' processor'. Also added a 'NUMBER_VALIDATOR' to StandardValidators

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

    $ git pull https://github.com/JPercivall/nifi NIFI-1582

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

    https://github.com/apache/nifi/pull/319.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 #319
    
----
commit 50f300cefdb98b60fd22dec1fdba7694064add96
Author: jpercivall <jo...@yahoo.com>
Date:   2016-04-01T21:27:42Z

    NIFI-1582 added state to UpdateAttribute as well as updated a few parts that hadn't be touched in years (referenced the 'FlowFileMetadataEnhancer' processor'. Also added a 'NUMBER_VALIDATOR' to StandardValidators

----


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as ...

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

    https://github.com/apache/nifi/pull/319#discussion_r91141036
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -162,20 +146,33 @@ public ValidationResult validate(String subject, String input, ValidationContext
         // static properties
         public static final PropertyDescriptor DELETE_ATTRIBUTES = new PropertyDescriptor.Builder()
                 .name("Delete Attributes Expression")
    -            .description("Regular expression for attributes to be deleted from flowfiles.")
    +            .description("Regular expression for attributes to be deleted from FlowFiles.")
                 .required(false)
                 .addValidator(DELETE_PROPERTY_VALIDATOR)
                 .expressionLanguageSupported(true)
                 .build();
     
    -    // relationships
    -    public static final Relationship REL_SUCCESS = new Relationship.Builder()
    -            .description("All FlowFiles are routed to this relationship").name("success").build();
    +    public static final PropertyDescriptor STORE_STATE = new PropertyDescriptor.Builder()
    +            .name("Store State")
    +            .description("Select whether or not state will be stored. Selecting 'Stateless' will offer the default functionality of purely updating the attributes on a " +
    +                    "FlowFile in a stateless manner. Selecting 'Stateful' will not only store the attributes on the FlowFile but also in the Processors state. See the 'Stateful Usage' " +
    +                    "topic of the 'Additional Details' section of this processor's documentation for more information")
    +            .required(true)
    +            .allowableValues(DO_NOT_STORE_STATE, STORE_STATE_LOCALLY)
    +            .defaultValue(DO_NOT_STORE_STATE)
    +            .build();
    +    public static final PropertyDescriptor STATEFUL_VARIABLES_INIT_VALUE = new PropertyDescriptor.Builder()
    +            .name("Stateful Variables Initial Value")
    +            .description("If using state to set/reference variables then this value is used to set the initial value of the stateful variable. This will only be used in the @OnScheduled method " +
    +                    "when state does not contain a value for the variable.")
    +            .required(false)
    +            .defaultValue("0")
    --- End diff --
    
    Should this be empty rather than zero? Seems like many of the use cases are numeric in nature so I see the value of a zero default, but wondering for the most general case if it should default to empty. I'm fine either way, just wanted your thoughts on it.


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as updated...

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

    https://github.com/apache/nifi/pull/319
  
    +1 LGTM, thanks for this awesome feature! I ran the tests and tried with various incrementing strategies, all looks good, merging to master


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as updated...

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

    https://github.com/apache/nifi/pull/319
  
    @mattyb149 removed the default value, added a validation check so that it is invalid if it is configured for state but has no initial value, and added a check in the unit tests.


---
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: NIFI-1582 added state to UpdateAttribute as wel...

Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the pull request:

    https://github.com/apache/nifi/pull/319#issuecomment-206472004
  
    @mattyb149 I did what you suggested, did a soft reset, re-committed (making a new hash like @apiri pointed out) and did a force push. It's rebuilding now


---
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: NIFI-1582 added state to UpdateAttribute as wel...

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

    https://github.com/apache/nifi/pull/319#discussion_r60083778
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -187,24 +167,83 @@ public UpdateAttribute() {
         protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
             List<PropertyDescriptor> descriptors = new ArrayList<>();
             descriptors.add(DELETE_ATTRIBUTES);
    +        descriptors.add(STORE_STATE);
    +        descriptors.add(STATEFUL_VARIABLES_INIT_VALUE);
             return Collections.unmodifiableList(descriptors);
         }
     
         @Override
         protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
    -        return new PropertyDescriptor.Builder()
    -                .name(propertyDescriptorName)
    -                .required(false)
    -                .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
    -                .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    -                .expressionLanguageSupported(true)
    -                .dynamic(true)
    -                .build();
    +        if(!stateful){
    +            return new PropertyDescriptor.Builder()
    +                    .name(propertyDescriptorName)
    +                    .required(false)
    +                    .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    +                    .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
    +                    .expressionLanguageSupported(true)
    +                    .dynamic(true)
    +                    .build();
    +        } else {
    +            return new PropertyDescriptor.Builder()
    +                    .name(propertyDescriptorName)
    +                    .required(false)
    +                    .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
    +                    .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    +                    .expressionLanguageSupported(true)
    +                    .dynamic(true)
    +                    .build();
    +        }
    --- End diff --
    
    That would be a cleaner approach. I will change. Thanks!


---
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: NIFI-1582 added state to UpdateAttribute as wel...

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

    https://github.com/apache/nifi/pull/319#discussion_r60060257
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -489,17 +546,32 @@ private FlowFile executeActions(final ProcessSession session, final ProcessConte
             final Map<String, String> attributesToUpdate = new HashMap<>(actions.size());
             final Set<String> attributesToDelete = new HashSet<>(actions.size());
     
    +        final Map<String, String> statefulAttributesToSet;
    +
    +        if (statefulAttributes != null){
    +            statefulAttributesToSet = new HashMap<>();
    +        } else {
    +            statefulAttributesToSet = null;
    +        }
    +
    +
             // go through each action
             for (final Action action : actions.values()) {
                 if (!action.getAttribute().equals(DELETE_ATTRIBUTES.getName())) {
    --- End diff --
    
    I didn't write this piece of code so I'm not entirely sure but I highly doubt it since its been on master for a while and hasn't been a problem so far.


---
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: NIFI-1582 added state to UpdateAttribute as wel...

Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on the pull request:

    https://github.com/apache/nifi/pull/319#issuecomment-206383566
  
    There was something that could be done, I can't remember. Maybe reset --soft HEAD^ and re-commit and push -f. Will give the same code a new commit SHA and should kick it off again. I thought it was simpler to do but maybe not. Also maybe we can re-run the Travis job manually?


---
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: NIFI-1582 added state to UpdateAttribute as wel...

Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the pull request:

    https://github.com/apache/nifi/pull/319#issuecomment-206383057
  
    @mattyb149 just tried pushing with no new commits and nothing happened. I believe I could push an empty commit but I don't think it's worth cluttering this PR with unnecessary commits. 


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as updated...

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

    https://github.com/apache/nifi/pull/319
  
    @JPercivall no issue. let us know when ready to review


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as updated...

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

    https://github.com/apache/nifi/pull/319
  
    @trixpan, Ah sorry for not commenting but it is ready for review. It is rebased and I added an EL function to access state in a consistent user-friendly way.


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as ...

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

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


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as ...

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

    https://github.com/apache/nifi/pull/319#discussion_r91142143
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -66,71 +72,49 @@
     import org.apache.nifi.update.attributes.Rule;
     import org.apache.nifi.update.attributes.serde.CriteriaSerDe;
     
    -/**
    --- End diff --
    
    Why was this removed? Because it's out of date, or the "official" doc is now in additionalDetails.html?


---
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: NIFI-1582 added state to UpdateAttribute as wel...

Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on the pull request:

    https://github.com/apache/nifi/pull/319#issuecomment-206094743
  
    I think you can re-push the PR (no new commits) and it might pick that up and rebuild.


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as updated...

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

    https://github.com/apache/nifi/pull/319
  
    Thanks @trixpan! Rgr, I'm working on rebasing this and making the state variable accessing a bit more user-friendly


---
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: NIFI-1582 added state to UpdateAttribute as wel...

Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the pull request:

    https://github.com/apache/nifi/pull/319#issuecomment-206094554
  
    Not sure why it failed on something unrelated on Travis it builds locally. Also I don't believe there is an easy way to rebuild the PR in Travis.


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as updated...

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

    https://github.com/apache/nifi/pull/319
  
    @JPercivall welcome back!
    
    Could you rebase this PR?


---
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: NIFI-1582 added state to UpdateAttribute as wel...

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

    https://github.com/apache/nifi/pull/319#discussion_r59996122
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -489,17 +546,32 @@ private FlowFile executeActions(final ProcessSession session, final ProcessConte
             final Map<String, String> attributesToUpdate = new HashMap<>(actions.size());
             final Set<String> attributesToDelete = new HashSet<>(actions.size());
     
    +        final Map<String, String> statefulAttributesToSet;
    +
    +        if (statefulAttributes != null){
    +            statefulAttributesToSet = new HashMap<>();
    +        } else {
    +            statefulAttributesToSet = null;
    +        }
    +
    +
             // go through each action
             for (final Action action : actions.values()) {
                 if (!action.getAttribute().equals(DELETE_ATTRIBUTES.getName())) {
                     try {
    -                    final String newAttributeValue = getPropertyValue(action.getValue(), context).evaluateAttributeExpressions(flowfile).getValue();
    +                    final String newAttributeValue = getPropertyValue(action.getValue(), context).evaluateAttributeExpressions(flowfile, statefulAttributes).getValue();
     
                         // log if appropriate
                         if (logger.isDebugEnabled()) {
                             logger.debug(String.format("%s setting attribute '%s' = '%s' for %s per rule '%s'.", this, action.getAttribute(), newAttributeValue, flowfile, ruleName));
                         }
     
    +                    if (statefulAttributesToSet != null) {
    +                        if(!action.getAttribute().equals("UpdateAttribute.matchedRule")) {
    --- End diff --
    
    Same comment as above on NPE


---
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: NIFI-1582 added state to UpdateAttribute as wel...

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

    https://github.com/apache/nifi/pull/319#discussion_r59995999
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -187,24 +167,83 @@ public UpdateAttribute() {
         protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
             List<PropertyDescriptor> descriptors = new ArrayList<>();
             descriptors.add(DELETE_ATTRIBUTES);
    +        descriptors.add(STORE_STATE);
    +        descriptors.add(STATEFUL_VARIABLES_INIT_VALUE);
             return Collections.unmodifiableList(descriptors);
         }
     
         @Override
         protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
    -        return new PropertyDescriptor.Builder()
    -                .name(propertyDescriptorName)
    -                .required(false)
    -                .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
    -                .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    -                .expressionLanguageSupported(true)
    -                .dynamic(true)
    -                .build();
    +        if(!stateful){
    +            return new PropertyDescriptor.Builder()
    +                    .name(propertyDescriptorName)
    +                    .required(false)
    +                    .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    +                    .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
    +                    .expressionLanguageSupported(true)
    +                    .dynamic(true)
    +                    .build();
    +        } else {
    +            return new PropertyDescriptor.Builder()
    +                    .name(propertyDescriptorName)
    +                    .required(false)
    +                    .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
    +                    .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    +                    .expressionLanguageSupported(true)
    +                    .dynamic(true)
    +                    .build();
    +        }
    --- End diff --
    
    Joe, instead of repeating the entire definition of the descriptor, have you considered a base builder and then finish descriptor based on condition. For example
    ```
    Builder somePropBuilder = new PropertyDescriptor.Builder()
                .name(propertyDescriptorName)
                .required(false)
                .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
                .expressionLanguageSupported(true)
                .dynamic(true);
    
    if(!stateful){
            return somePropBuilder
                   .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
                   .build();
    } else {
           return somePropBuilder
                   .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
                   .build();
    }
    ```



---
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: NIFI-1582 added state to UpdateAttribute as wel...

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/319#issuecomment-206384283
  
    There are no facilities to manually start a Travis job.  You could likely squash your two commits and push those.  The core thing is that a new hash is seen for the Travis webhooks, I believe.


---
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: NIFI-1582 added state to UpdateAttribute as wel...

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

    https://github.com/apache/nifi/pull/319#discussion_r60060311
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -187,24 +167,83 @@ public UpdateAttribute() {
         protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
             List<PropertyDescriptor> descriptors = new ArrayList<>();
             descriptors.add(DELETE_ATTRIBUTES);
    +        descriptors.add(STORE_STATE);
    +        descriptors.add(STATEFUL_VARIABLES_INIT_VALUE);
             return Collections.unmodifiableList(descriptors);
         }
     
         @Override
         protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
    -        return new PropertyDescriptor.Builder()
    -                .name(propertyDescriptorName)
    -                .required(false)
    -                .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
    -                .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    -                .expressionLanguageSupported(true)
    -                .dynamic(true)
    -                .build();
    +        if(!stateful){
    +            return new PropertyDescriptor.Builder()
    +                    .name(propertyDescriptorName)
    +                    .required(false)
    +                    .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    +                    .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
    +                    .expressionLanguageSupported(true)
    +                    .dynamic(true)
    +                    .build();
    +        } else {
    +            return new PropertyDescriptor.Builder()
    +                    .name(propertyDescriptorName)
    +                    .required(false)
    +                    .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
    +                    .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    +                    .expressionLanguageSupported(true)
    +                    .dynamic(true)
    +                    .build();
    +        }
    +    }
    +
    +    @Override
    +    public void onPropertyModified(final PropertyDescriptor descriptor, final String oldValue, final String newValue) {
    +        super.onPropertyModified(descriptor, oldValue, newValue);
    +
    +        if (descriptor.equals(STORE_STATE)) {
    +            if ("true".equalsIgnoreCase(newValue)) {
    +                stateful = true;
    +            } else {
    +                stateful = false;
    +            }
    +        }
    --- End diff --
    
    Yup, 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 issue #319: NIFI-1582 added state to UpdateAttribute as well as updated...

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

    https://github.com/apache/nifi/pull/319
  
    @mattyb149 pushed out a new commit adding documentation on the init value requirements and clarification on the "empty" value. 
    
    Thanks for reviewing documentation too! I totally forgot it.


---
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: NIFI-1582 added state to UpdateAttribute as wel...

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

    https://github.com/apache/nifi/pull/319#discussion_r59996108
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -489,17 +546,32 @@ private FlowFile executeActions(final ProcessSession session, final ProcessConte
             final Map<String, String> attributesToUpdate = new HashMap<>(actions.size());
             final Set<String> attributesToDelete = new HashSet<>(actions.size());
     
    +        final Map<String, String> statefulAttributesToSet;
    +
    +        if (statefulAttributes != null){
    +            statefulAttributesToSet = new HashMap<>();
    +        } else {
    +            statefulAttributesToSet = null;
    +        }
    +
    +
             // go through each action
             for (final Action action : actions.values()) {
                 if (!action.getAttribute().equals(DELETE_ATTRIBUTES.getName())) {
    --- End diff --
    
    Any possibility of getAttribute() returning null? Just want to avoid unnecessary NPE.


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as ...

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

    https://github.com/apache/nifi/pull/319#discussion_r91147897
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -162,20 +146,33 @@ public ValidationResult validate(String subject, String input, ValidationContext
         // static properties
         public static final PropertyDescriptor DELETE_ATTRIBUTES = new PropertyDescriptor.Builder()
                 .name("Delete Attributes Expression")
    -            .description("Regular expression for attributes to be deleted from flowfiles.")
    +            .description("Regular expression for attributes to be deleted from FlowFiles.")
                 .required(false)
                 .addValidator(DELETE_PROPERTY_VALIDATOR)
                 .expressionLanguageSupported(true)
                 .build();
     
    -    // relationships
    -    public static final Relationship REL_SUCCESS = new Relationship.Builder()
    -            .description("All FlowFiles are routed to this relationship").name("success").build();
    +    public static final PropertyDescriptor STORE_STATE = new PropertyDescriptor.Builder()
    +            .name("Store State")
    +            .description("Select whether or not state will be stored. Selecting 'Stateless' will offer the default functionality of purely updating the attributes on a " +
    +                    "FlowFile in a stateless manner. Selecting 'Stateful' will not only store the attributes on the FlowFile but also in the Processors state. See the 'Stateful Usage' " +
    +                    "topic of the 'Additional Details' section of this processor's documentation for more information")
    +            .required(true)
    +            .allowableValues(DO_NOT_STORE_STATE, STORE_STATE_LOCALLY)
    +            .defaultValue(DO_NOT_STORE_STATE)
    +            .build();
    +    public static final PropertyDescriptor STATEFUL_VARIABLES_INIT_VALUE = new PropertyDescriptor.Builder()
    +            .name("Stateful Variables Initial Value")
    +            .description("If using state to set/reference variables then this value is used to set the initial value of the stateful variable. This will only be used in the @OnScheduled method " +
    +                    "when state does not contain a value for the variable.")
    +            .required(false)
    +            .defaultValue("0")
    --- End diff --
    
    Yeah I agree that it should be empty. I will change


---
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: NIFI-1582 added state to UpdateAttribute as wel...

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

    https://github.com/apache/nifi/pull/319#discussion_r59996046
  
    --- Diff: nifi-nar-bundles/nifi-update-attribute-bundle/nifi-update-attribute-processor/src/main/java/org/apache/nifi/processors/attributes/UpdateAttribute.java ---
    @@ -187,24 +167,83 @@ public UpdateAttribute() {
         protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
             List<PropertyDescriptor> descriptors = new ArrayList<>();
             descriptors.add(DELETE_ATTRIBUTES);
    +        descriptors.add(STORE_STATE);
    +        descriptors.add(STATEFUL_VARIABLES_INIT_VALUE);
             return Collections.unmodifiableList(descriptors);
         }
     
         @Override
         protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
    -        return new PropertyDescriptor.Builder()
    -                .name(propertyDescriptorName)
    -                .required(false)
    -                .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
    -                .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    -                .expressionLanguageSupported(true)
    -                .dynamic(true)
    -                .build();
    +        if(!stateful){
    +            return new PropertyDescriptor.Builder()
    +                    .name(propertyDescriptorName)
    +                    .required(false)
    +                    .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    +                    .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
    +                    .expressionLanguageSupported(true)
    +                    .dynamic(true)
    +                    .build();
    +        } else {
    +            return new PropertyDescriptor.Builder()
    +                    .name(propertyDescriptorName)
    +                    .required(false)
    +                    .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING, true))
    +                    .addValidator(StandardValidators.ATTRIBUTE_KEY_PROPERTY_NAME_VALIDATOR)
    +                    .expressionLanguageSupported(true)
    +                    .dynamic(true)
    +                    .build();
    +        }
    +    }
    +
    +    @Override
    +    public void onPropertyModified(final PropertyDescriptor descriptor, final String oldValue, final String newValue) {
    +        super.onPropertyModified(descriptor, oldValue, newValue);
    +
    +        if (descriptor.equals(STORE_STATE)) {
    +            if ("true".equalsIgnoreCase(newValue)) {
    +                stateful = true;
    +            } else {
    +                stateful = false;
    +            }
    +        }
    --- End diff --
    
    Isn't it the same as saying 
    ```
    stateful = "true".equalsIgnoreCase(newValue);
    ```


---
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 #319: NIFI-1582 added state to UpdateAttribute as well as updated...

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

    https://github.com/apache/nifi/pull/319
  
    Hey @trixpan, if you have free cycles I'd appreciate finalizing this review. I've rebased on the latest master


---
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.
---