NIFI-11123: fix default value and update docs #6899

KalmanJantner · 2023-01-27T15:42:23Z

Summary

NIFI-11123

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Apache NiFi Jira issue created

Pull Request Tracking

Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

Pull Request based on current revision of the main branch
Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

Build completed using mvn clean install -P contrib-check
- JDK 8
- JDK 11
- JDK 17

Licensing

New dependencies are compatible with the Apache License 2.0 according to the License Policy
New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

Documentation formatting appears as expected in rendered files

exceptionfactory

Thanks for the documentation updates @KalmanJantner. Changing the example value property introduces a Parameter Value reference, as opposed to the current FlowFile attribute reference. Neither one may be provided, so having that JSON snippet as the default value does not seem like the best approach. Recommending Parameter usage in the additional details is one thing, but it should also be possible configure the Processor without reference to a Parameter. Perhaps the default JSON value should be removed if there is no suitable generic value that can be obtained from a FlowFile attribute.

exceptionfactory · 2023-01-31T22:33:07Z

One additional note, these changes should reference a new Jira issue, not the existing Jira issue, which is already resolved.

tpalfy · 2023-02-01T16:53:49Z

Instead of relying on a parameter context for the output bucket we could add a property on the processor.
For that property and the incoming flowfile attributes we could build a custom map and add that as parameter to the evaluateAttributeExpression call when getting the value of the payload JSON.
When both the property and a similar flowfile attribute is set, flowfile attributes should have precedence.

Also the type of Vision action (like DOCUMENT_TEXT_DETECTION) could come from a property as well.

KalmanJantner · 2023-02-01T20:11:04Z

One additional note, these changes should reference a new Jira issue, not the existing Jira issue, which is already resolved.

Thanks, I have created a new ticket and updated the PR desc and title according to that.

KalmanJantner · 2023-02-01T20:11:45Z

Instead of relying on a parameter context for the output bucket we could add a property on the processor. For that property and the incoming flowfile attributes we could build a custom map and add that as parameter to the evaluateAttributeExpression call when getting the value of the payload JSON. When both the property and a similar flowfile attribute is set, flowfile attributes should have precedence.

Also the type of Vision action (like DOCUMENT_TEXT_DETECTION) could come from a property as well.

Thank you for the suggestion. I have updated the PR based on your suggestion.

exceptionfactory

Thanks for the updates @KalmanJantner. The general approach looks good, but I recommended some additions to the new property descriptions, and style adjustments.

...ors/src/main/java/org/apache/nifi/processors/gcp/vision/AbstractStartGcpVisionOperation.java

…java/org/apache/nifi/processors/gcp/vision/AbstractStartGcpVisionOperation.java Co-authored-by: exceptionfactory <exceptionfactory@apache.org>

tpalfy · 2023-02-02T14:31:18Z

LGTM
@exceptionfactory if you are okay with the recent changes I'll merge into main.

exceptionfactory

Thanks for the updates @KalmanJantner!

This looks just about ready to go @tpalfy, I noticed that the default value and examples need to be updated to match the adjusted vision-feature-type. With those changes, it looks good.

exceptionfactory · 2023-02-02T14:33:45Z

...rc/main/java/org/apache/nifi/processors/gcp/vision/StartGcpVisionAnnotateFilesOperation.java

@@ -56,12 +56,12 @@ public class StartGcpVisionAnnotateFilesOperation extends AbstractStartGcpVision
                    "                \"mimeType\": \"application/pdf\"\n" +
                    "            },\n" +
                    "            \"features\": [{\n" +
-                    "                    \"type\": \"DOCUMENT_TEXT_DETECTION\",\n" +
+                    "                    \"type\": \"${feature-type}\",\n" +


This needs to be adjusted to vision-feature-type:

Suggested change

" \"type\": \"${feature-type}\",\n" +

" \"type\": \"${vision-feature-type}\",\n" +

Thanks fixed.

exceptionfactory · 2023-02-02T14:33:57Z

...c/main/java/org/apache/nifi/processors/gcp/vision/StartGcpVisionAnnotateImagesOperation.java

@@ -55,21 +55,21 @@ public class StartGcpVisionAnnotateImagesOperation extends AbstractStartGcpVisio
                    "            }\n" +
                    "        },\n" +
                    "        \"features\": [{\n" +
-                    "            \"type\": \"FACE_DETECTION\",\n" +
+                    "            \"type\": \"${feature-type}\",\n" +


Suggested change

" \"type\": \"${feature-type}\",\n" +

" \"type\": \"${vision-feature-type}\",\n" +

Thanks fixed.

exceptionfactory · 2023-02-02T14:34:04Z

...pache.nifi.processors.gcp.vision.StartGcpVisionAnnotateFilesOperation/additionalDetails.html

@@ -55,12 +55,12 @@ <h3>Payload</h3>
                "mimeType": "application/pdf"
            },
            "features": [{
-                    "type": "DOCUMENT_TEXT_DETECTION",
+                    "type": "${feature-type}",


Suggested change

"type": "${feature-type}",

"type": "${vision-feature-type}",

Thanks fixed.

exceptionfactory · 2023-02-02T14:34:13Z

...ache.nifi.processors.gcp.vision.StartGcpVisionAnnotateImagesOperation/additionalDetails.html

@@ -53,13 +53,13 @@ <h3>Payload</h3>
 			}
 		},
 		"features": [{
-			"type": "DOCUMENT_TEXT_DETECTION",
+			"type": "${feature-type}",


Suggested change

"type": "${feature-type}",

"type": "${vision-feature-type}",

Thanks fixed.

tpalfy · 2023-02-02T17:50:48Z

LGTM
Thank you for your work @KalmanJantner and @exceptionfactory for your review!
Merged into main.

exceptionfactory requested changes Jan 31, 2023

View reviewed changes

KalmanJantner changed the title ~~NIFI-10953: fix default value and update docs~~ NIFI-11123: fix default value and update docs Feb 1, 2023

NIFI-11123: fix default value and update docs

20ec193

KalmanJantner force-pushed the NIFI-10953-default-value-fix branch from 504f471 to 20ec193 Compare February 1, 2023 20:09

exceptionfactory reviewed Feb 1, 2023

View reviewed changes

KalmanJantner and others added 6 commits February 2, 2023 06:45

Update nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/…

439d4b9

…java/org/apache/nifi/processors/gcp/vision/AbstractStartGcpVisionOperation.java Co-authored-by: exceptionfactory <exceptionfactory@apache.org>

Update nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/…

5a34254

…java/org/apache/nifi/processors/gcp/vision/AbstractStartGcpVisionOperation.java Co-authored-by: exceptionfactory <exceptionfactory@apache.org>

Update nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/…

3d10ed1

…java/org/apache/nifi/processors/gcp/vision/AbstractStartGcpVisionOperation.java Co-authored-by: exceptionfactory <exceptionfactory@apache.org>

Update nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/…

de6b153

…java/org/apache/nifi/processors/gcp/vision/AbstractStartGcpVisionOperation.java Co-authored-by: exceptionfactory <exceptionfactory@apache.org>

Update nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/…

2727f24

…java/org/apache/nifi/processors/gcp/vision/AbstractStartGcpVisionOperation.java Co-authored-by: exceptionfactory <exceptionfactory@apache.org>

NIFI-11123: fix default value and update docs - apply review comments

b96a098

KalmanJantner requested a review from exceptionfactory February 2, 2023 11:17

exceptionfactory reviewed Feb 2, 2023

View reviewed changes

NIFI-11123: fix default value and update docs - apply review comments

555ffeb

KalmanJantner requested a review from exceptionfactory February 2, 2023 15:48

asfgit closed this in 027e2b9 Feb 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NIFI-11123: fix default value and update docs #6899

NIFI-11123: fix default value and update docs #6899

KalmanJantner commented Jan 27, 2023 •

edited

exceptionfactory left a comment

exceptionfactory commented Jan 31, 2023

tpalfy commented Feb 1, 2023

KalmanJantner commented Feb 1, 2023

KalmanJantner commented Feb 1, 2023

exceptionfactory left a comment

tpalfy commented Feb 2, 2023

exceptionfactory left a comment

exceptionfactory Feb 2, 2023

KalmanJantner Feb 2, 2023

exceptionfactory Feb 2, 2023

KalmanJantner Feb 2, 2023

exceptionfactory Feb 2, 2023

KalmanJantner Feb 2, 2023

exceptionfactory Feb 2, 2023

KalmanJantner Feb 2, 2023

tpalfy commented Feb 2, 2023

	" \"type\": \"${feature-type}\",\n" +
	" \"type\": \"${vision-feature-type}\",\n" +

NIFI-11123: fix default value and update docs #6899

NIFI-11123: fix default value and update docs #6899

Conversation

KalmanJantner commented Jan 27, 2023 • edited

Summary

Tracking

Issue Tracking

Pull Request Tracking

Pull Request Formatting

Verification

Build

Licensing

Documentation

exceptionfactory left a comment

Choose a reason for hiding this comment

exceptionfactory commented Jan 31, 2023

tpalfy commented Feb 1, 2023

KalmanJantner commented Feb 1, 2023

KalmanJantner commented Feb 1, 2023

exceptionfactory left a comment

Choose a reason for hiding this comment

tpalfy commented Feb 2, 2023

exceptionfactory left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpalfy commented Feb 2, 2023

KalmanJantner commented Jan 27, 2023 •

edited