[GitHub] [nifi] turcsanyip commented on a diff in pull request #6373: NIFI-10411 Add record processing feature to PublishMQTT processor

[GitBox <gi...@apache.org>]

turcsanyip commented on code in PR #6373:
URL: https://github.com/apache/nifi/pull/6373#discussion_r965334984


##########
nifi-nar-bundles/nifi-mqtt-bundle/nifi-mqtt-processors/src/main/java/org/apache/nifi/processors/mqtt/common/AbstractMQTTProcessor.java:
##########
@@ -232,6 +235,22 @@ private static String getSupportedSchemeList() {
             .addValidator(StandardValidators.POSITIVE_INTEGER_VALIDATOR)
             .build();
 
+    public static final PropertyDescriptor RECORD_READER = new PropertyDescriptor.Builder()
+            .name("record-reader")
+            .displayName("Record Reader")
+            .description("The Record Reader to use for received messages")

Review Comment:
   The descriptions for the Record Reader/Writer were inherited from the Consume processor but they are not exactly applicable to the Publish processor because it does not receive messages and does not write FlowFiles but the opposite (reads FF and writes message).
   
   I would avoid changing it to a common but generic (and meaningless) message and would rather suggest the following:
   - do not specify description in the abstract class
   - redeclare the PropertyDescriptor in Consume and Publish using `new PropertyDescriptor.Builder().fromPropertyDescriptor()` and add the specific descriptions there



##########
nifi-nar-bundles/nifi-mqtt-bundle/nifi-mqtt-processors/pom.xml:
##########
@@ -93,5 +93,11 @@
             <artifactId>nifi-schema-registry-service-api</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.glassfish</groupId>
+            <artifactId>javax.json</artifactId>
+            <version>1.0.4</version>
+            <scope>test</scope>
+        </dependency>

Review Comment:
   We more commonly use Jackson for Json processing in the NiFi code base. Would it be possible to use this library here too?



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