nandorsoma commented on code in PR #6417:
URL: https://github.com/apache/nifi/pull/6417#discussion_r970768112
##########
nifi-nar-bundles/nifi-mqtt-bundle/nifi-mqtt-processors/src/main/java/org/apache/nifi/processors/mqtt/PublishMQTT.java:
##########
@@ -117,23 +118,32 @@ public class PublishMQTT extends AbstractMQTTProcessor
implements MqttCallback {
.description("FlowFiles that failed to send to the destination are
transferred to this relationship.")
.build();
- private static final List<PropertyDescriptor> descriptors;
- private static final Set<Relationship> relationships;
-
- static {
- final List<PropertyDescriptor> innerDescriptorsList =
getAbstractPropertyDescriptors();
- innerDescriptorsList.add(PROP_TOPIC);
- innerDescriptorsList.add(PROP_QOS);
- innerDescriptorsList.add(PROP_RETAIN);
- innerDescriptorsList.add(RECORD_READER);
- innerDescriptorsList.add(RECORD_WRITER);
- descriptors = Collections.unmodifiableList(innerDescriptorsList);
-
- final Set<Relationship> innerRelationshipsSet = new HashSet<>();
- innerRelationshipsSet.add(REL_SUCCESS);
- innerRelationshipsSet.add(REL_FAILURE);
- relationships = Collections.unmodifiableSet(innerRelationshipsSet);
- }
+ private static final List<PropertyDescriptor> PROPERTIES =
Collections.unmodifiableList(Arrays.asList(
+ PROP_BROKER_URI,
+ PROP_MQTT_VERSION,
+ PROP_USERNAME,
+ PROP_PASSWORD,
+ PROP_SSL_CONTEXT_SERVICE,
+ PROP_CLEAN_SESSION,
+ PROP_SESSION_EXPIRY_INTERVAL,
+ PROP_CLIENTID,
+ PROP_TOPIC,
+ PROP_RETAIN,
+ PROP_QOS,
+ RECORD_READER,
Review Comment:
I'd probably put reader/writer properties to the end of the list to keep the
mqtt related properties together.
##########
nifi-nar-bundles/nifi-mqtt-bundle/nifi-mqtt-processors/src/main/java/org/apache/nifi/processors/mqtt/ConsumeMQTT.java:
##########
@@ -214,26 +216,35 @@ public class ConsumeMQTT extends AbstractMQTTProcessor
implements MqttCallback {
.autoTerminateDefault(true) // to make sure flow are still valid
after upgrades
.build();
- private static final List<PropertyDescriptor> descriptors;
- private static final Set<Relationship> relationships;
-
- static {
- final List<PropertyDescriptor> innerDescriptorsList =
getAbstractPropertyDescriptors();
- innerDescriptorsList.add(PROP_GROUPID);
- innerDescriptorsList.add(PROP_TOPIC_FILTER);
- innerDescriptorsList.add(PROP_QOS);
- innerDescriptorsList.add(PROP_MAX_QUEUE_SIZE);
- innerDescriptorsList.add(ADD_ATTRIBUTES_AS_FIELDS);
- innerDescriptorsList.add(MESSAGE_DEMARCATOR);
- innerDescriptorsList.add(RECORD_READER);
- innerDescriptorsList.add(RECORD_WRITER);
- descriptors = Collections.unmodifiableList(innerDescriptorsList);
-
- final Set<Relationship> innerRelationshipsSet = new HashSet<>();
- innerRelationshipsSet.add(REL_MESSAGE);
- innerRelationshipsSet.add(REL_PARSE_FAILURE);
- relationships = Collections.unmodifiableSet(innerRelationshipsSet);
- }
+ private static final List<PropertyDescriptor> PROPERTIES =
Collections.unmodifiableList(Arrays.asList(
+ PROP_BROKER_URI,
+ PROP_MQTT_VERSION,
+ PROP_USERNAME,
+ PROP_PASSWORD,
+ PROP_SSL_CONTEXT_SERVICE,
+ PROP_CLEAN_SESSION,
+ PROP_SESSION_EXPIRY_INTERVAL,
+ PROP_CLIENTID,
+ PROP_GROUPID,
+ PROP_TOPIC_FILTER,
+ PROP_QOS,
+ PROP_MAX_QUEUE_SIZE,
Review Comment:
I'd put this property to the end of the list (before reader/writers if you
accept that comment) because this property is not strictly related to the MQTT
client itself as the other properties around it.
##########
nifi-nar-bundles/nifi-mqtt-bundle/nifi-mqtt-processors/src/main/java/org/apache/nifi/processors/mqtt/ConsumeMQTT.java:
##########
@@ -161,15 +162,26 @@ public class ConsumeMQTT extends AbstractMQTTProcessor
implements MqttCallback {
.addValidator(StandardValidators.POSITIVE_INTEGER_VALIDATOR)
.build();
+ public static final PropertyDescriptor RECORD_READER = new
PropertyDescriptor.Builder()
+ .fromPropertyDescriptor(BASE_RECORD_READER)
+ .description("The Record Reader to use for parsing received MQTT
Messages into Records.")
+ .build();
+
+ public static final PropertyDescriptor RECORD_WRITER = new
PropertyDescriptor.Builder()
+ .fromPropertyDescriptor(BASE_RECORD_WRITER)
+ .description("The Record Writer to use for serializing Records
before writing them to a FlowFile.")
+ .build();
+
public static final PropertyDescriptor ADD_ATTRIBUTES_AS_FIELDS = new
PropertyDescriptor.Builder()
.name("add-attributes-as-fields")
.displayName("Add attributes as fields")
- .description("If using the Records reader/writer and if setting
this property to true, default fields "
+ .description("If using the Record Reader/Writer and if setting
this property to true, default fields "
Review Comment:
Because of the dependsOn property, the first if will always be true. I'd
remove it to make the description simpler.
##########
nifi-nar-bundles/nifi-mqtt-bundle/nifi-mqtt-processors/src/main/java/org/apache/nifi/processors/mqtt/common/AbstractMQTTProcessor.java:
##########
@@ -307,7 +277,7 @@ public Collection<ValidationResult> customValidate(final
ValidationContext valid
final boolean readerIsSet =
validationContext.getProperty(BASE_RECORD_READER).isSet();
final boolean writerIsSet =
validationContext.getProperty(BASE_RECORD_WRITER).isSet();
if ((readerIsSet && !writerIsSet) || (!readerIsSet && writerIsSet)) {
- results.add(new ValidationResult.Builder().subject("Reader and
Writer").valid(false)
+ results.add(new ValidationResult.Builder().subject("Record Reader
and Writer").valid(false)
.explanation("both Record Reader and Writer must be set
when used.").build());
Review Comment:
Rendered text on the UI looks like "Record Reader and Writer is invalid,
because both Record Reader and Writer must be set when used." which is a bit
verbose. I'd remove the reference to "Record Reader and Writer" from the
explanation to make the description simpler.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
