exceptionfactory commented on code in PR #7463:
URL: https://github.com/apache/nifi/pull/7463#discussion_r1256525303
##########
nifi-nar-bundles/nifi-poi-bundle/nifi-poi-processors/src/main/java/org/apache/nifi/processors/poi/ConvertExcelToCSVProcessor.java:
##########
@@ -317,6 +335,7 @@ private void handleExcelSheet(ProcessSession session,
FlowFile originalParentFF,
session.transfer(ff, SUCCESS);
} catch (RuntimeException e) {
+ getLogger().error("Failed to process Excel sheet: " +
e.getMessage(), e);
Review Comment:
String concatenation should not be used for log messages, and the exception
message would be included in the stack trace. Recommend either simplifying the
message, or perhaps introducing a placeholder for the FlowFile or some other
identifying property.
```suggestion
getLogger().error("Failed to process Excel sheet", e);
```
##########
nifi-nar-bundles/nifi-poi-bundle/nifi-poi-processors/src/main/java/org/apache/nifi/processors/poi/ConvertExcelToCSVProcessor.java:
##########
@@ -199,6 +213,10 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
final int firstRow =
context.getProperty(ROWS_TO_SKIP).evaluateAttributeExpressions(flowFile).asInteger()
- 1;
final List<Integer> columnsToSkip = getColumnsToSkip(context,
flowFile);
+ // Set min inflate ratio before loading documents
+ final float minInflateRatio =
context.getProperty(MIN_INFLATE_RATIO).evaluateAttributeExpressions(flowFile).asFloat();
+ ZipSecureFile.setMinInflateRatio(minInflateRatio);
Review Comment:
This appears to set the value in a global way, not specific to the instance
of the Processor. Is there another way to set the property for the individual
instance of the reader?
--
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]
