kfaraz commented on code in PR #18547:
URL: https://github.com/apache/druid/pull/18547#discussion_r2374875724
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -626,52 +631,64 @@ public Enumerable<Object[]> scan(DataContext root)
/**
* Returns a row for all node types which don't serve data. The returned
row contains only static information.
*/
- private static Object[] buildRowForNonDataServer(DiscoveryDruidNode
discoveryDruidNode)
+ private Object[] buildRowForNonDataServer(DiscoveryDruidNode
discoveryDruidNode)
{
final DruidNode node = discoveryDruidNode.getDruidNode();
- return new Object[]{
- node.getHostAndPortToUse(),
- node.getHost(),
- (long) node.getPlaintextPort(),
- (long) node.getTlsPort(),
- StringUtils.toLowerCase(discoveryDruidNode.getNodeRole().toString()),
- null,
- UNKNOWN_SIZE,
- UNKNOWN_SIZE,
- null,
- toStringOrNull(discoveryDruidNode.getStartTime())
- };
+ try {
+ return new Object[]{
+ node.getHostAndPortToUse(),
+ node.getHost(),
+ (long) node.getPlaintextPort(),
+ (long) node.getTlsPort(),
+
StringUtils.toLowerCase(discoveryDruidNode.getNodeRole().toString()),
+ null,
+ UNKNOWN_SIZE,
+ UNKNOWN_SIZE,
+ null,
+ toStringOrNull(discoveryDruidNode.getStartTime()),
+ node.getLabels() == null ? null :
jsonMapper.writeValueAsString(node.getLabels())
+ };
+ }
+ catch (JsonProcessingException e) {
+ throw new RuntimeException(e);
+ }
}
/**
* Returns a row for all node types which don't serve data. The returned
row contains only static information.
*/
- private static Object[] buildRowForNonDataServerWithLeadership(
+ private Object[] buildRowForNonDataServerWithLeadership(
DiscoveryDruidNode discoveryDruidNode,
boolean isLeader
)
{
final DruidNode node = discoveryDruidNode.getDruidNode();
- return new Object[]{
- node.getHostAndPortToUse(),
- node.getHost(),
- (long) node.getPlaintextPort(),
- (long) node.getTlsPort(),
- StringUtils.toLowerCase(discoveryDruidNode.getNodeRole().toString()),
- null,
- UNKNOWN_SIZE,
- UNKNOWN_SIZE,
- isLeader ? 1L : 0L,
- toStringOrNull(discoveryDruidNode.getStartTime())
- };
+ try {
Review Comment:
Rather than adding the try-catch in the 2 places, better to add a new method
in `JacksonUtils` named `writeValueAsString()` which catches the
`JsonProcessingException` and throws a `DruidException` instead. It will be
useful for other places in the code too.
##########
sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java:
##########
@@ -1515,6 +1533,9 @@ private static void verifyTypes(final List<Object[]>
rows, final RowSignature si
case STRING:
expectedClass = String.class;
break;
+ case COMPLEX:
+ expectedClass = Map.class;
+ break;
Review Comment:
Is this still needed?
##########
sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java:
##########
@@ -396,7 +396,7 @@ public void setUp(@TempDir File tmpDir) throws Exception
);
private final DiscoveryDruidNode overlord = new DiscoveryDruidNode(
- new DruidNode("s2", "localhost", false, 8090, null, true, false),
+ new DruidNode("s2", "localhost", false, 8090, null, null, true, false,
ImmutableMap.of("overlordKey", "overlordValue")),
Review Comment:
Nit: Consider using `Map.of()` instead of `ImmutableMap.of()` for brevity.
--
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]
