Github user joewitt commented on the pull request:
https://github.com/apache/nifi/pull/335#issuecomment-207181902
Hello - thanks for contributing. Some feedback:
- The commit message and the PR will need to associate to some open JIRA.
If the JIRA were say NIFI-XYZ1 then the commit message should have "NIFI-XYZ1
...the message"
- The batch size does documentation does need to be changed. The change to
a new default number of 16KB is fine but changing the name of the property is
not ideal as it would break compatibility. You can change the display name
safely but it is already appropriate.
- The description you gave is good.
- I like your change to using the data size validator but the problem is
this would make all existing configurations invalid as they'd need to add the
unit to their value. I am +0 on changing that personally as we do have
provisions to deal with it and can document it in a migration guide. However,
thinking further on it I'm told on older versions of Kafka that batch size
really was about the number of messages so changing it may not be ideal.
You could choose to simply fix the description of the property and leave
the rest for now. There is an actively under work PR/JIRA to move to
supporting the 0.9 Kafka producer and consumer API and that will give us a
chance to tighten some of these behaviors up as well. It will be two new
processors and these will still be around for some time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
