Skip to content

Conversation

@sashapolo
Copy link
Contributor

No description provided.

@sashapolo sashapolo force-pushed the ignite-14382 branch 8 times, most recently from 8728335 to f25821a Compare April 5, 2021 16:33
Copy link
Contributor

@sergey-chugunov-1985 sergey-chugunov-1985 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a minor comment with naming suggestion.

@sashapolo sashapolo force-pushed the ignite-14382 branch 2 times, most recently from 77ddca5 to ad4e183 Compare April 6, 2021 09:18
@sashapolo sashapolo marked this pull request as ready for review April 6, 2021 10:16
/** {@inheritDoc} */
@Override public void addMessageHandler(NetworkMessageHandler handler) {
messageHandlers.add(handler);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a way to unregister message handlers for dynamically started components.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on the use cases please, because I may be out of context for this suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some components (like a cache) can start/stop dynamically and should have possibility to deregister related listeners on stop.
I'm ok to implement this as a separate ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asfgit asfgit closed this in fcfe1a6 Apr 12, 2021
@sashapolo sashapolo deleted the ignite-14382 branch April 12, 2021 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants