NIFI-14190 Add Max String Length to ValidationJson#9660
NIFI-14190 Add Max String Length to ValidationJson#9660exceptionfactory merged 2 commits intoapache:mainfrom
Conversation
e8949d2 to
1dfa759
Compare
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for the contribution @NimrodAvni! The overall approach looks good, I just noted two convention-related things, and otherwise this should be good.
| ); | ||
|
|
||
| private static final ObjectMapper MAPPER = new ObjectMapper().configure(JsonParser.Feature.ALLOW_COMMENTS, true); | ||
| private ObjectMapper MAPPER; |
There was a problem hiding this comment.
The variable name should be lowercased to mapper since it is no longer static.
| private ObjectMapper MAPPER; | |
| private ObjectMapper mapper; |
There was a problem hiding this comment.
@exceptionfactory I was wondering whether this should be done in light of the changes you made in #9648. Do want ObjectMapper to be an instance variable in this processor?
There was a problem hiding this comment.
Thanks @dan-s1. This does result in creating an instance of ObjectMapper per Processor instance, but since it is created in onScheduled it is the correct place for initialization.
|
|
||
| public static final PropertyDescriptor MAX_STRING_LENGTH = new PropertyDescriptor.Builder() | ||
| .name("Max String Length") | ||
| .displayName("Max String Length") |
There was a problem hiding this comment.
Since this is a new property, the displayName is not needed.
| .displayName("Max String Length") |
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for making the adjustments @NimrodAvni, the latest version looks good! +1 merging
Summary
NIFI-14190
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation