markap14 commented on pull request #5324: URL: https://github.com/apache/nifi/pull/5324#issuecomment-1034006358
@markobean @joewitt thanks for your responses. So here's my personal viewpoint. - I do agree that we have several things already like `jsonPath` functions in Expression Language, escapeHtml, etc. That should never have existed. - I do agree also that there could be some applicable use cases. I can imagine having some metadata that is say < 1 KB of XML that could be in an attribute. It's probably rare but definitely feasible. - My concern here is that by enabling this, we may further perpetuate the idea that it's ok to put arbitrarily large chunks of data in attributes. - @markobean I know you have significant experience here and it probably makes perfect sense for your use case. However, the more I interact with users, the more that I see this pattern of attributes are keyed content, and it creates a lot of issues. While it probably makes perfect sense for your use case, I'm concerned about encouraging those with less experience. - The annotation does provide some warning. - The processor itself isn't designed to create large attributes from content - only verify that already exists as an attribute is, in fact, valid. Without it, the solution may be to simply create the attribute anyway and avoid performing any validation on it, potentially leading to errors later in the flow. I personally don't like adding this in such that we further encourage poor flow design patterns. That said, these are well thought-out, well-articulated arguments. While I won't vote a +1 for this issue & merge the PR, I won't vote a -1 either. I'll say I'm a +0. Fair enough? @exceptionfactory if you are comfortable with the changes, you should feel free to merge from my standpoint. -- 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]
