[
https://issues.apache.org/jira/browse/NIFI-4504?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16304965#comment-16304965
]
ASF GitHub Bot commented on NIFI-4504:
--------------------------------------
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?
> SimpleMapCache/PersistentMapCache: Add removeAndGet and removeByPatternAndGet
> -----------------------------------------------------------------------------
>
> Key: NIFI-4504
> URL: https://issues.apache.org/jira/browse/NIFI-4504
> Project: Apache NiFi
> Issue Type: Improvement
> Affects Versions: 1.4.0
> Reporter: Jon Kessler
> Assignee: Michael Moser
> Priority: Minor
>
> Typical map implementations return the value that was removed when performing
> a remove. Because you couldn't update the existing remove methods without it
> being a breaking change I suggest adding new versions of the remove and
> removeByPattern methods that return the removed value(s).
> These changes should also be applied up the chain to any class that makes use
> of these classes such as the MapCacheServer and
> AtomicDistributedMapCacheClient.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
