dan-s1 commented on PR #11208:
URL: https://github.com/apache/nifi/pull/11208#issuecomment-4408180275
@exceptionfactory I have made all the changes you requested. I am concerned
though that the configuration for the `YamlTreeReader` is now awkward since it
extends `JsonTreeReader`. The properties `Max String Length` and `Parsing
Strategy` do not really make sense since for Yaml parsing it really does not
support these equivalent options. Even the original `Allow Comment `does not
make sense since SnakeYaml (the implementation we use for Yaml parsing)
supports parsing comments that is when one plans to preserve them, but by
default it ignores comments. Hence since we are not preserving them we
shouldn't need any option to keep them. This leads to awkward code as I would
like exclude these property descriptors in `YamlTreeReader` but the only way I
can remove them is with code like this:
```
@Override
protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
final List<PropertyDescriptor> supportedPropertyDescriptors = new
ArrayList<>(super.getSupportedPropertyDescriptors());
supportedPropertyDescriptors.remove(AbstractJsonRowRecordReader.MAX_STRING_LENGTH);
supportedPropertyDescriptors.remove(AbstractJsonRowRecordReader.PARSING_STRATEGY);
return supportedPropertyDescriptors;
}
```
The whole reason we had `YamlTreeReader` extends `JsonTreeReader` was to
not have to duplicate code as the JSON code could drive the Yaml parsing. But
there are parts like these properties we do not want. Would the above code be
acceptable?
--
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]
