-
Notifications
You must be signed in to change notification settings - Fork 18
fix: cpu and ram columns fallback #3211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| content={ | ||
| <DefinitionList responsive> | ||
| {row.PoolStats.map((pool) => | ||
| {row.PoolStats?.map((pool) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we check row.PoolStats in upper code - looks like ? is redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mislooked - my bad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 file reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the CPU column's fallback behavior to properly handle missing PoolStats data by allowing the column to display CPU usage calculated from CoresUsed and CoresTotal when available. Previously, the column would immediately show a placeholder when PoolStats was missing, even if alternative data sources were available.
Key Changes
- Removed early return when
PoolStatsis missing, allowing calculation fromCoresUsed/CoresTotal - Added conditional check to only calculate from
PoolStatswhen it exists - Disabled popover tooltip when
PoolStatsis unavailable - Added optional chaining for safer array mapping
Comments suppressed due to low confidence (1)
src/components/nodesColumns/columns.tsx:251
- Potential division by zero when totalThreadsCount is 0. This can happen if PoolStats is an empty array or if all pools have 0 threads. The division would result in Infinity or NaN. Consider adding a check to ensure totalThreadsCount is greater than 0 before performing the division, or provide a fallback value.
if (totalPoolUsage === undefined && row.PoolStats) {
let totalThreadsCount = 0;
totalPoolUsage = row.PoolStats.reduce((acc, pool) => {
totalThreadsCount += Number(pool.Threads);
return acc + Number(pool.Usage) * Number(pool.Threads);
}, 0);
totalPoolUsage = totalPoolUsage / totalThreadsCount;
closes #2546
Greptile Overview
Greptile Summary
This PR makes the CPU column behavior consistent with the RAM column by displaying "no data" when
PoolStatsis unavailable, instead of showing a dash.Key changes:
PoolStatswas missingPoolStatsexistsPoolStatsis unavailablePoolStats?.map()to safely handle undefined valuesProgressViewercomponent now properly displays "no data" whentotalPoolUsageisundefinedConfidence Score: 5/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant Table as Nodes Table participant Column as getCpuColumn participant Cell as CellWithPopover participant Progress as ProgressViewer Table->>Column: render({row}) alt CoresUsed and CoresTotal available Column->>Column: Calculate totalPoolUsage from cores else PoolStats available Column->>Column: Calculate totalPoolUsage from PoolStats else No data available Column->>Column: totalPoolUsage = undefined end Column->>Cell: Create CellWithPopover Cell->>Cell: Set disabled={!row.PoolStats} Column->>Progress: Pass totalPoolUsage alt totalPoolUsage is numeric Progress->>Table: Display progress bar else totalPoolUsage is undefined Progress->>Table: Display "no data" endCI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: ✅
Current: 62.48 MB | Main: 62.48 MB
Diff: 0.04 KB (-0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information