You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "hr-ireh (via GitHub)" <gi...@apache.org> on 2025/03/06 06:29:08 UTC

[PR] Bug with task number NIFI-13738 has been fixed. [nifi]

hr-ireh opened a new pull request, #9776:
URL: https://github.com/apache/nifi/pull/9776

   setMaxInboundMessageBodySize property would be configurable as a NiFi UI property. 


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


Re: [PR] NIFI-13738 Add Max Inbound Message Body Size Property for AMQP [nifi]

Posted by "Liammarwood (via GitHub)" <gi...@apache.org>.
Liammarwood commented on PR #9776:
URL: https://github.com/apache/nifi/pull/9776#issuecomment-2874807153

   This seemed to have broken the PublishAMQP processor now as its expecting the Max property but there is no Validators for it 


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


Re: [PR] NIFI-13738 Add Max Inbound Message Body Size Property for AMQP [nifi]

Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on PR #9776:
URL: https://github.com/apache/nifi/pull/9776#issuecomment-2706606503

   The checkstyle validation needs to be fixed.
   
   Regarding the property, what I'm saying is that you can remove it from the PROPERTY_DESCRIPTORS list of the Abstract class and be added into the PROPERTY_DESCRIPTORS list of the Consume processor. Everything else remains unchanged. This only affects where the property will be visible in the UI.


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


Re: [PR] NIFI-13738 Add Max Inbound Message Body Size Property for AMQP [nifi]

Posted by "hr-ireh (via GitHub)" <gi...@apache.org>.
hr-ireh commented on PR #9776:
URL: https://github.com/apache/nifi/pull/9776#issuecomment-2706573791

   It is true that the settings are related to the consumer, but like the port and IP, they must be set in the ConnectionFactory, which is created in the AbstractAMQPProcessor.
   How to set MaxInboundMessageBodySize in ConsumeAMQP!


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


Re: [PR] NIFI-13738 Add Max Inbound Message Body Size Property for AMQP [nifi]

Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #9776:
URL: https://github.com/apache/nifi/pull/9776#discussion_r1983008109


##########
nifi-extension-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java:
##########
@@ -303,7 +311,8 @@ protected Connection createConnection(ProcessContext context, ExecutorService ex
         final ConnectionFactory cf = new ConnectionFactory();
         cf.setUsername(context.getProperty(USER).evaluateAttributeExpressions().getValue());
         cf.setPassword(context.getProperty(PASSWORD).getValue());
-
+        cf.setMaxInboundMessageBodySize(Integer.parseInt(context.getProperty(MAX_INBOUND_MESSAGE_BODY_SIZE).evaluateAttributeExpressions().getValue()));
+        

Review Comment:
   Then you can do something like:
   
   ````java
   .addValidator(StandardValidators.createDataSizeBoundsValidator(1, Integer.MAX_VALUE))
   ````
   
   Using the data size approach makes it more user friendly as the user can just provide something like '1 MB', etc.



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


Re: [PR] NIFI-13738 Add Max Inbound Message Body Size Property for AMQP [nifi]

Posted by "hr-ireh (via GitHub)" <gi...@apache.org>.
hr-ireh commented on code in PR #9776:
URL: https://github.com/apache/nifi/pull/9776#discussion_r1982973063


##########
nifi-extension-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java:
##########
@@ -303,7 +311,8 @@ protected Connection createConnection(ProcessContext context, ExecutorService ex
         final ConnectionFactory cf = new ConnectionFactory();
         cf.setUsername(context.getProperty(USER).evaluateAttributeExpressions().getValue());
         cf.setPassword(context.getProperty(PASSWORD).getValue());
-
+        cf.setMaxInboundMessageBodySize(Integer.parseInt(context.getProperty(MAX_INBOUND_MESSAGE_BODY_SIZE).evaluateAttributeExpressions().getValue()));
+        

Review Comment:
   MaxInboundMessageBodySize is an integer and we need to validate that the input is not out of range.
   
   My suggestion is to use this:
   StandardValidators.POSITIVE_INTEGER_VALIDATOR



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


Re: [PR] NIFI-13738 Add Max Inbound Message Body Size Property for AMQP [nifi]

Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 closed pull request #9776: NIFI-13738 Add Max Inbound Message Body Size Property for AMQP
URL: https://github.com/apache/nifi/pull/9776


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


Re: [PR] NIFI-13738 Add Max Inbound Message Body Size Property for AMQP [nifi]

Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #9776:
URL: https://github.com/apache/nifi/pull/9776#discussion_r1982937355


##########
nifi-extension-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java:
##########
@@ -303,7 +311,8 @@ protected Connection createConnection(ProcessContext context, ExecutorService ex
         final ConnectionFactory cf = new ConnectionFactory();
         cf.setUsername(context.getProperty(USER).evaluateAttributeExpressions().getValue());
         cf.setPassword(context.getProperty(PASSWORD).getValue());
-
+        cf.setMaxInboundMessageBodySize(Integer.parseInt(context.getProperty(MAX_INBOUND_MESSAGE_BODY_SIZE).evaluateAttributeExpressions().getValue()));
+        

Review Comment:
   ```suggestion
           cf.setMaxInboundMessageBodySize(context.getProperty(MAX_INBOUND_MESSAGE_BODY_SIZE).evaluateAttributeExpressions().asDataSize(DataUnit.B).intValue());
   
   ```



##########
nifi-extension-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java:
##########
@@ -138,7 +138,14 @@ abstract class AbstractAMQPProcessor<T extends AMQPWorker> extends AbstractProce
             .allowableValues("true", "false")
             .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
             .build();
-
+    public static final PropertyDescriptor MAX_INBOUND_MESSAGE_BODY_SIZE = new PropertyDescriptor.Builder()
+            .name("MaxInboundMessageBodySize")
+            .description("Maximum body size of inbound (received) messages in bytes.")
+            .required(false)
+            .defaultValue("67108864")
+            .expressionLanguageSupported(ExpressionLanguageScope.ENVIRONMENT)
+            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+            .build();

Review Comment:
   Recommend using built-in data size validator:
   
   ```suggestion
       public static final PropertyDescriptor MAX_INBOUND_MESSAGE_BODY_SIZE = new PropertyDescriptor.Builder()
               .name("Max Inbound Message Body Size")
               .description("Maximum body size of inbound (received) messages.")
               .required(true)
               .defaultValue("64 MB")
               .expressionLanguageSupported(ExpressionLanguageScope.ENVIRONMENT)
               .addValidator(StandardValidators.DATA_SIZE_VALIDATOR)
               .build();
   ```



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


Re: [PR] NIFI-13738 Add Max Inbound Message Body Size Property for AMQP [nifi]

Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #9776:
URL: https://github.com/apache/nifi/pull/9776#discussion_r1983043416


##########
nifi-extension-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java:
##########
@@ -303,7 +311,8 @@ protected Connection createConnection(ProcessContext context, ExecutorService ex
         final ConnectionFactory cf = new ConnectionFactory();
         cf.setUsername(context.getProperty(USER).evaluateAttributeExpressions().getValue());
         cf.setPassword(context.getProperty(PASSWORD).getValue());
-
+        cf.setMaxInboundMessageBodySize(context.getProperty(MAX_INBOUND_MESSAGE_BODY_SIZE).evaluateAttributeExpressions().asDataSize(DataUnit.B).intValue());
+        

Review Comment:
   Please make sure the build is valid with `mvn clean install -Pcontrib-check`
   
   ````
   Warning:  src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java:[315] (regexp) RegexpSinglelineJava: Line has trailing whitespace.
   ````
   
   ```suggestion
           cf.setMaxInboundMessageBodySize(context.getProperty(MAX_INBOUND_MESSAGE_BODY_SIZE).evaluateAttributeExpressions().asDataSize(DataUnit.B).intValue());
   
   ```



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