Adding an ability to use a default implementation for sending history.#5347
Adding an ability to use a default implementation for sending history.#5347dmitrii-beliakov wants to merge 3 commits intodevelopfrom
Conversation
| private transient Integer taskId; | ||
|
|
||
| @Getter(AccessLevel.PROTECTED) | ||
| @Getter(AccessLevel.PUBLIC) |
There was a problem hiding this comment.
Just out of curiosity, you decided to point out explicitly AccessLevel.PUBLIC only to keep the common behaviour here, like as it is in the cases for AccessLevel.PROTECTED?
There was a problem hiding this comment.
It is a part of an interface now, so it has to be public
| default String getHistoryStreamName() { | ||
| return "HUB_TO_HISTORY_TOPOLOGY_SENDER"; | ||
| } |
There was a problem hiding this comment.
Would it be more convenient to keep HUB_TO_HISTORY_TOPOLOGY_SENDER constant in the Stream enum?
There was a problem hiding this comment.
Yes, I have a thought that it needs to be in some place that is accessible in the bolts and the service at the same time. We need this constant when configuring topologies, so it would be convenient to have some unified approach of adding. If it is a helper method then it can't be enum that we have in the topology package.
This is a task to think about. But once it is done, we can change this method without changing the rest of the code.
There was a problem hiding this comment.
I think getHistoryStreamName() must be used in declareOutputFields
I extracted interfaces from AbstractBolt, this allows to have an interface with the default implementation for sending history. I chose it to be an interface and not a class in the hierarchy of bolts, so that we can opt-in or opt-out the default implementation more easily. Also it was needed to move classes to another module, up in the dependency structure.
I added the usage to one bolt to show how it is going to look like.
Although is solves code duplication problem, there are some downsides: