Conversation
smcvb
left a comment
smcvb
left a comment
There was a problem hiding this comment.
Minor request to add a replace operation.
| * @param tags the tags to remove from the event | ||
| * @return a CompletableFuture containing the response | ||
| */ | ||
| CompletableFuture<RemoveTagsResponse> removeTags(long sequence, Collection<Tag> tags); |
There was a problem hiding this comment.
Nit: Can we add a replace on this interface, that defaults to first invoke remove and then add? It would keep the Axon Server API simple, but provide an easy step for consumers of the Axon Server Connector.
There was a problem hiding this comment.
This is an interesting suggestion! However, if we don't support replace in the Axon Server, it wouldn't make sense to support it in the connector. The main reason is that we cannot make the operation atomic in the connector. Hence, it's better to leave error handling to the end client rather than have the connector try to do it automatically.
I don't know when we'll support replace in the Axon Server.
There was a problem hiding this comment.
Ah, so a replace is scheduled, that's curious.
I figured that from the connector's perspective, it doesn't have to be atomic. The documentation can clearly state it's a two-step operation, using the existing addTags and removeTags.
Furthermore, exception handling...wouldn't that just be "removing fails, so we skip adding..?"
If I am reasoning about this to simplistically, just let me know!
There was a problem hiding this comment.
It's not scheduled, but there are some thoughts about it. Especially if it's even necessary, or even if that's something worthwhile.
Your reasoning is sound! Personally, I'd prefer not to add it, because (without reading the docs) it is expected to be atomic. I'd add it only when AS supports it to avoid any confusion - it'd create more questions than usefulness.
There was a problem hiding this comment.
Fair points, @m1l4n54v1c! Let's drop this idea then.
The request mainly comes from anticipating how users will want to integrate with this.
Assuming that Axon Framework will have some integration, I'd expect people would want a replace functionality; hence why I was looking for it.
However, that's all in the future, no necessity for now whatsoever.
smcvb
left a comment
smcvb
left a comment
There was a problem hiding this comment.
My concerns have been addressed, hence I'm approving this pull request.
4 participants
This PR supports the Retagging feature in Axon Server. Since the feature is still unavailable in the Axon Server, it is in draft mode.