You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "exceptionfactory (via GitHub)" <gi...@apache.org> on 2023/05/10 15:41:44 UTC

[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7228: NIFI-11524 Documentation and config experience improvement for ExecuteStreamCommand

exceptionfactory commented on code in PR #7228:
URL: https://github.com/apache/nifi/pull/7228#discussion_r1190095727


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/resources/docs/org.apache.nifi.processors.standard.ExecuteStreamCommand/additionalDetails.html:
##########
@@ -0,0 +1,166 @@
+<!DOCTYPE html>
+<html lang="en">
+    <!--
+        Licensed to the Apache Software Foundation (ASF) under one or more
+        contributor license agreements.  See the NOTICE file distributed with
+        this work for additional information regarding copyright ownership.
+        The ASF licenses this file to You under the Apache License, Version 2.0
+        (the "License"); you may not use this file except in compliance with
+        the License.  You may obtain a copy of the License at
+        http://www.apache.org/licenses/LICENSE-2.0
+        Unless required by applicable law or agreed to in writing, software
+        distributed under the License is distributed on an "AS IS" BASIS,
+        WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+        See the License for the specific language governing permissions and
+        limitations under the License.
+    -->
+    <head>
+        <meta charset="utf-8" />
+        <title>ExecuteStreamCommand</title>
+        <link rel="stylesheet" href="../../../../../css/component-usage.css" type="text/css" />
+    </head>
+
+    <body>
+        <h2>Description</h2>
+        <p>The ExecuteStreamCommand processor provides a flexible way to integrate external commands and scripts into NiFi data flows.
+        ExecuteStreamCommand can pass the incoming FlowFile's content to the command that it executes similarly how piping works.</p>
+
+        <h2>Configuration options</h2>
+
+        <h3>Working Directory</h3>
+        <p>If not specified, NiFi root will be the default working directory.</p>
+
+        <h3>Configuring command arguments</h3>
+        <p>The ExecuteStreamCommand processor provides two ways to specify command arguments: using Dynamic Properties and the Command Arguments field.</p>
+
+        <h4>Command Arguments field</h4>
+        <p>This is the default. If there are multiple arguments, they need to be separated by a character specified in the Argument Delimiter field.
+        When needed, '-' and '--' can be provided, but in these cases Argument Delimiter should be a different character.</p>
+
+        <p>Consider that we want to list all files in a directory which is different from the working directory:</p>
+
+        <table>
+            <tr>
+                <th>Command Path</th>
+                <th>Command Arguments</th>
+            </tr>
+            <tr>
+                <td>ls</td>
+                <td>-lah;/path/to/dir</td>
+            </tr>
+        </table>
+
+        <p><b>NOTE:</b> the command should be on <code>$PATH</code> or it should be in the working directory, otherwise path also should be specified.</p>
+
+        <h4>Dynamic Properties</h4>
+        <p>Arguments can be specified with Dynamic Properties. Dynamic Properties with the pattern of 'command.arguments.&lt;commandIndex&gt;' will be appended
+        to the command in ascending order.</p>
+
+        <p>The above example with dynamic properties would look like this:</p>
+
+        <table>
+            <tr>
+                <th>Property Name</th>
+                <th>Property Value</th>
+            </tr>
+            <tr>
+                <td>command.arguments.0</td>
+                <td>-lah</td>
+            </tr>
+            <tr>
+                <td>command.arguments.1</td>
+                <td>/path/to/dir</td>
+            </tr>
+        </table>
+
+        <h3>Configuring environment variables</h3>
+        <p>In addition to specifying command arguments using the Command Argument field or Dynamic Properties, users can also use environment variables with
+        the ExecuteStreamCommand processor. Environment variables are a set of key-value pairs that can be accessed by processes running on the system.
+        ExecuteStreamCommand will treat every Dynamic Property as an environment variable that doesn't match the pattern 'command.arguments.&lt;commandIndex&gt;'.</p>
+
+        <p>Consider that we want to execute a maven command with the processor. If there are multiple Java versions installed on the system, you can specify

Review Comment:
   `Maven` should be capitalized.
   ```suggestion
           <p>Consider that we want to execute a Maven command with the processor. If there are multiple Java versions installed on the system, you can specify
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/resources/docs/org.apache.nifi.processors.standard.ExecuteStreamCommand/additionalDetails.html:
##########
@@ -0,0 +1,166 @@
+<!DOCTYPE html>
+<html lang="en">
+    <!--
+        Licensed to the Apache Software Foundation (ASF) under one or more
+        contributor license agreements.  See the NOTICE file distributed with
+        this work for additional information regarding copyright ownership.
+        The ASF licenses this file to You under the Apache License, Version 2.0
+        (the "License"); you may not use this file except in compliance with
+        the License.  You may obtain a copy of the License at
+        http://www.apache.org/licenses/LICENSE-2.0
+        Unless required by applicable law or agreed to in writing, software
+        distributed under the License is distributed on an "AS IS" BASIS,
+        WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+        See the License for the specific language governing permissions and
+        limitations under the License.
+    -->
+    <head>
+        <meta charset="utf-8" />
+        <title>ExecuteStreamCommand</title>
+        <link rel="stylesheet" href="../../../../../css/component-usage.css" type="text/css" />
+    </head>
+
+    <body>
+        <h2>Description</h2>
+        <p>The ExecuteStreamCommand processor provides a flexible way to integrate external commands and scripts into NiFi data flows.
+        ExecuteStreamCommand can pass the incoming FlowFile's content to the command that it executes similarly how piping works.</p>
+
+        <h2>Configuration options</h2>
+
+        <h3>Working Directory</h3>
+        <p>If not specified, NiFi root will be the default working directory.</p>
+
+        <h3>Configuring command arguments</h3>
+        <p>The ExecuteStreamCommand processor provides two ways to specify command arguments: using Dynamic Properties and the Command Arguments field.</p>
+
+        <h4>Command Arguments field</h4>
+        <p>This is the default. If there are multiple arguments, they need to be separated by a character specified in the Argument Delimiter field.
+        When needed, '-' and '--' can be provided, but in these cases Argument Delimiter should be a different character.</p>
+
+        <p>Consider that we want to list all files in a directory which is different from the working directory:</p>
+
+        <table>
+            <tr>
+                <th>Command Path</th>
+                <th>Command Arguments</th>
+            </tr>
+            <tr>
+                <td>ls</td>
+                <td>-lah;/path/to/dir</td>
+            </tr>
+        </table>
+
+        <p><b>NOTE:</b> the command should be on <code>$PATH</code> or it should be in the working directory, otherwise path also should be specified.</p>
+
+        <h4>Dynamic Properties</h4>
+        <p>Arguments can be specified with Dynamic Properties. Dynamic Properties with the pattern of 'command.arguments.&lt;commandIndex&gt;' will be appended
+        to the command in ascending order.</p>
+
+        <p>The above example with dynamic properties would look like this:</p>
+
+        <table>
+            <tr>
+                <th>Property Name</th>
+                <th>Property Value</th>
+            </tr>
+            <tr>
+                <td>command.arguments.0</td>
+                <td>-lah</td>
+            </tr>
+            <tr>
+                <td>command.arguments.1</td>
+                <td>/path/to/dir</td>
+            </tr>
+        </table>
+
+        <h3>Configuring environment variables</h3>
+        <p>In addition to specifying command arguments using the Command Argument field or Dynamic Properties, users can also use environment variables with
+        the ExecuteStreamCommand processor. Environment variables are a set of key-value pairs that can be accessed by processes running on the system.
+        ExecuteStreamCommand will treat every Dynamic Property as an environment variable that doesn't match the pattern 'command.arguments.&lt;commandIndex&gt;'.</p>
+
+        <p>Consider that we want to execute a maven command with the processor. If there are multiple Java versions installed on the system, you can specify
+        which version will be used by setting the <code>JAVA_HOME</code> environment variable. The output FlowFile will looke like this if we run
+        <code>mvn</code> command with <code>--version</code> argument:</p>
+
+        <code>
+            <pre style="overflow:scroll">
+Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)
+Maven home: /path/to/maven/home
+Java version: 11.0.18, vendor: Eclipse Adoptium, runtime: /path/to/default/java/home
+Default locale: en_US, platform encoding: UTF-8
+OS name: "mac os x", version: "13.1", arch: "x86_64", family: "mac"
+            </pre>
+        </code>
+
+        <table>
+            <tr>
+                <th>Property Name</th>
+                <th>Property Value</th>
+            </tr>
+            <tr>
+                <td>JAVA_HOME</td>
+                <td>path/to/another/java/home</td>
+            </tr>
+        </table>
+
+        <p>After specifying the <code>JAVA_HOME</code> property, you can notice that maven is using a different runtime:</p>
+        <code>
+            <pre>
+Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)
+Maven home: /path/to/maven/home
+Java version: 11.0.18, vendor: Eclipse Adoptium, runtime: /path/to/another/java/home
+Default locale: en_US, platform encoding: UTF-8
+OS name: "mac os x", version: "13.1", arch: "x86_64", family: "mac"
+            </pre>
+        </code>
+
+        <h3>Streaming input to the command</h3>
+        <p>ExecuteStreamCommand passes the incoming FlowFile's content to the command that it executes similarly how piping works.
+        It is possible to disable this behavior with the Ignore STDIN property. In the above examples we didn't use the
+        incoming FlowFile's content, so in this cases we could leverage this property for additional performance gain.</p>

Review Comment:
   ```suggestion
           incoming FlowFile's content, so in this case we could leverage this property for additional performance gain.</p>
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteStreamCommand.java:
##########
@@ -363,8 +348,37 @@ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String
                     .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
                     .addValidator(ATTRIBUTE_EXPRESSION_LANGUAGE_VALIDATOR)
                     .build();
+        } else {
+            return new PropertyDescriptor.Builder()
+                    .name(propertyDescriptorName)
+                    .description("Sets the environment variable '" + propertyDescriptorName + "' for the process' environment")
+                    .dynamic(true)
+                    .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+                    .build();
         }
-        return null;
+    }
+
+    @Override
+    protected Collection<ValidationResult> customValidate(ValidationContext validationContext) {
+        final List<ValidationResult> validationResults = new ArrayList<>(super.customValidate(validationContext));
+
+        final String argumentStrategy = validationContext.getProperty(ARGUMENTS_STRATEGY).getValue();
+        if (DYNAMIC_PROPERTY_ARGUMENTS_STRATEGY.getValue() != argumentStrategy) {
+            for (final PropertyDescriptor propertyDescriptor : validationContext.getProperties().keySet()) {
+                if (!propertyDescriptor.isDynamic()) {
+                    continue;
+                }
+
+                final String propertyName = propertyDescriptor.getName();
+                final Matcher matcher = COMMAND_ARGUMENT_PATTERN.matcher(propertyName);
+                if (matcher.matches()) {
+                    logger.warn(String.format("'%s' should be set to '%s' when command arguments are supplied as Dynamic Properties. The property '%s' will be ignored.",

Review Comment:
   That sounds like a good approach. For this pull request, `String.format()` should not be used for the log message, instead `{}` placeholders should be used.
   ```suggestion
                       logger.warn("[{}] should be set to [{}] when command arguments are supplied as Dynamic Properties. The property [{}] will be ignored.",
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org