NissimShiman commented on PR #6151: URL: https://github.com/apache/nifi/pull/6151#issuecomment-1185732426
@markobean Thank you very much for taking time tp look at this and for your very thought out and preceptive observations. I plan to take care of the first one. The second one is also valid but it looks like it fixing it will end up exposing other issues that may start to push this into a larger scope. Specifically, making this change will cause https://github.com/apache/nifi/blob/rel/nifi-1.16.3/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java#L785 to fail There seems to be historical support for ignoring buffer sizes when regex of .* is used. See https://github.com/apache/nifi/blob/rel/nifi-1.16.3/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java#L276 which seems to allude to this as well. In reality, the maximum buffer size is still looked at just like any other (i.e. and a minimum of 8KB is always assumed here as well), so the underlying code is currently not supporting this functionality, so your wondering about the 8KB default buffer size is valid, but perhaps resolving that should be done in another ticket (to handle the cascading need for some reimplementation of support for the .* regex) The third observation is also valid and I plan to update the javadocs to capture your point, although I think we should keep the IOException as it can be thrown from https://github.com/apache/nifi/blob/rel/nifi-1.16.3/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/stream/io/util/AbstractTextDemarcator.java#L129 Thank you again @markobean ! -- 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]
