Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2284
@mosermw Thank you for updating the PR, it's much easier to review now. The
code implemented at DistributedMapCacheClientService and MapCacheServer looks
good to me. Travis error should be fine as the failure looks depending on
execution condition.
However, let me ask one more time since adding methods is easy but hard to
remove afterwards for things like this. Are all of these three methods,
`removeAndGet`, `removeByPatternAndGet` and `keySet` required by the folks you
know of? I prefer to minimize the addition as those methods are only supported
by DistributedMapCacheClientService, which means those will not be used by most
developers who write Processors. Also, it confuses such developers which to
choose from `remove` and `removeAndGet`. Probably I imagine these concerns are
what made you struggle with implementing default method to just throw
UnsupportedOperationException.
If there is no significant needs for `removeAndGet` and
`removeByPatternAndGet`, then I'd prefer omit these method from
DistributedMapCacheClient interface. As long as we support `keySet`, pretty
much everything can be done at the caller side. The only benefit to have AndGet
methods I am aware of is reducing the network traffic, and atomicity (atomicity
is not that important in these method IMHO). Do you think those are more
important than adding mostly unsupported methods into a common interface?
---
