Copilot commented on code in PR #18613:
URL: https://github.com/apache/druid/pull/18613#discussion_r2425103343
##########
web-console/src/views/services-view/services-view.tsx:
##########
@@ -629,6 +637,37 @@ ORDER BY
Cell: this.renderFilterableCell('version'),
Aggregated: () => '',
},
+ {
+ Header: 'CPU processors',
+ show: visibleColumns.shown('CPU processors'),
+ accessor: 'available_processors',
+ className: 'padded',
+ filterable: false,
+ width: 120,
+ Cell: ({ value }) => value,
Review Comment:
The CPU processors cell renderer should handle null/undefined values
consistently with other columns. Consider adding a null check similar to the
total memory column.
```suggestion
Cell: ({ value }) => (value == null ? '' : value),
```
##########
web-console/src/views/services-view/services-view.tsx:
##########
@@ -629,6 +637,37 @@ ORDER BY
Cell: this.renderFilterableCell('version'),
Aggregated: () => '',
},
+ {
+ Header: 'CPU processors',
+ show: visibleColumns.shown('CPU processors'),
+ accessor: 'available_processors',
+ className: 'padded',
+ filterable: false,
+ width: 120,
+ Cell: ({ value }) => value,
+ Aggregated: ({ subRows }) => {
+ const originalRows: ServiceResultRow[] = subRows.map(r =>
r._original);
Review Comment:
Inconsistent type annotation between CPU processors and total memory
aggregation functions. Both should use explicit type annotation for consistency.
##########
web-console/src/views/services-view/services-view.tsx:
##########
@@ -629,6 +637,37 @@ ORDER BY
Cell: this.renderFilterableCell('version'),
Aggregated: () => '',
},
+ {
+ Header: 'CPU processors',
+ show: visibleColumns.shown('CPU processors'),
+ accessor: 'available_processors',
+ className: 'padded',
+ filterable: false,
+ width: 120,
+ Cell: ({ value }) => value,
+ Aggregated: ({ subRows }) => {
+ const originalRows: ServiceResultRow[] = subRows.map(r =>
r._original);
+ const totalAvailableProcessors = sum(originalRows, s =>
s.available_processors);
+ return totalAvailableProcessors;
+ },
+ },
+ {
+ Header: 'Total memory',
+ show: visibleColumns.shown('Total memory'),
+ accessor: 'total_memory',
+ className: 'padded',
+ width: 120,
+ filterable: false,
+ Cell: ({ value }) => {
+ if (value === null) return '';
+ return formatBytes(value, true);
+ },
+ Aggregated: ({ subRows }) => {
+ const originalRows = subRows.map(r => r._original);
Review Comment:
Inconsistent type annotation between CPU processors and total memory
aggregation functions. Both should use explicit type annotation for consistency.
```suggestion
const originalRows: ServiceResultRow[] = subRows.map(r =>
r._original);
```
##########
server/src/main/java/org/apache/druid/discovery/DiscoveryDruidNode.java:
##########
@@ -88,6 +108,10 @@ public DiscoveryDruidNode(
this.services.putAll(services);
}
this.startTime = startTime;
+
+ // Happens if service is running older version of Druid
+ this.availableProcessors = availableProcessors != null ?
availableProcessors : -1;
+ this.totalMemory = totalMemory != null ? totalMemory : -1;
Review Comment:
Magic number -1 used for default values should be defined as named constants
to improve code clarity and maintainability.
--
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]
