dan-s1 commented on code in PR #8224:
URL: https://github.com/apache/nifi/pull/8224#discussion_r1446422220
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestValidateCsv.java:
##########
@@ -200,7 +200,7 @@ public void testStrlenStrMinMaxStrRegex() {
runner.run();
runner.assertTransferCount(ValidateCsv.REL_INVALID, 1);
runner.getFlowFilesForRelationship(ValidateCsv.REL_INVALID).get(0).assertAttributeEquals("validation.error.message",
- "'testapache.org' does not match the regular expression
'[a-z0-9\\._]+@[a-z0-9\\.]+'");
+ "'testapache.org' does not match the regular expression
'[a-z0-9\\._]+@[a-z0-9\\.]+' {lineNo=1, rowNo=1, columnNo=3}");
Review Comment:
Per earlier comments
```suggestion
"'testapache.org' does not match the regular expression
'[a-z0-9\\._]+@[a-z0-9\\.]+' at {line=1, row=1, column=3}");
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestValidateCsv.java:
##########
@@ -122,7 +122,7 @@ public void testValidDateOptionalDouble() {
runner.run();
runner.assertTransferCount(ValidateCsv.REL_INVALID, 1);
runner.getFlowFilesForRelationship(ValidateCsv.REL_INVALID).get(0).assertAttributeEquals("validation.error.message",
- "'22/111954' could not be parsed as a Date");
+ "'22/111954' could not be parsed as a Date {lineNo=1, rowNo=1,
columnNo=2}");
Review Comment:
Per earlier comments
```suggestion
"'22/111954' could not be parsed as a Date at {line=1,
row=1, column=2}");
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ValidateCsv.java:
##########
@@ -514,8 +514,11 @@ public void process(OutputStream out) throws IOException {
} catch (final SuperCsvException e) {
valid.set(false);
+ final String coordinates =
String.format("{lineNo=%d, rowNo=%d, columnNo=%d}",
e.getCsvContext().getLineNumber(),
Review Comment:
This maybe a personal preference but I do not think the `No` suffix is
necessary
```suggestion
final String coordinates =
String.format("{line=%d, row=%d, column=%d}", e.getCsvContext().getLineNumber(),
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ValidateCsv.java:
##########
@@ -514,8 +514,11 @@ public void process(OutputStream out) throws IOException {
} catch (final SuperCsvException e) {
valid.set(false);
+ final String coordinates =
String.format("{lineNo=%d, rowNo=%d, columnNo=%d}",
e.getCsvContext().getLineNumber(),
+ e.getCsvContext().getRowNumber(),
e.getCsvContext().getColumnNumber());
+ final String errorMessage =
e.getLocalizedMessage() + " " + coordinates;
Review Comment:
Again maybe a personal preference but adding the word ` at` gives more
context to the coordinates string
```suggestion
final String errorMessage =
e.getLocalizedMessage() + " at " + coordinates;
```
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
