clintropolis commented on code in PR #13930:
URL: https://github.com/apache/druid/pull/13930#discussion_r1140685776
##########
server/src/main/java/org/apache/druid/server/coordination/ServerType.java:
##########
@@ -149,6 +149,18 @@ public static ServerType fromString(String type)
return ServerType.valueOf(StringUtils.toUpperCase(type).replace('-', '_'));
}
+ public static ServerType fromNodeRole(NodeRole nodeRole)
+ {
+ // this doesn't actually check that the NodeRole is a typical data node
+ if (nodeRole.equals(NodeRole.HISTORICAL)) {
+ return ServerType.HISTORICAL;
+ } else if (nodeRole.equals(NodeRole.BROKER)) {
+ return ServerType.BROKER;
+ } else {
+ return ServerType.INDEXER_EXECUTOR;
Review Comment:
Yeah, I thought about that. Nothing _should_ be using `ServerType` unless
they are dealing with nodes that can hold segments so its quite unexpected that
other node roles would end up here. I was hesitant to add another precondition
here because throwing any sort of exception in the callback that is using this
method is pretty uncool, so if we add stricter validation here then the callers
should probably either do their own `NodeRole` check or catch the exceptions
(just in case improper node roles somehow get in the watch list).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
