-
Notifications
You must be signed in to change notification settings - Fork 17
feat: dont fetch render for false graphshard #2803
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
|
/claude_review |
|
Claude finished @astandrik's task —— View job Code Review for PR #2803: Don't fetch render for false graphshardReview Checklist
SummaryThis PR improves performance by preventing unnecessary chart data fetches when the GraphShard capability is explicitly false. The changes are well-implemented and follow the codebase standards. ✅ Strengths
📋 Observations
🎯 No Issues FoundThe implementation is clean, follows all project standards, and achieves the performance optimization goal without introducing any issues. Performance ImpactThe changes will reduce unnecessary API calls and improve performance for clusters without GraphShard capability, which aligns with the issue #2801 requirements. |
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 optimizes chart rendering behavior by preventing unnecessary data fetching when GraphShard capability is explicitly disabled. The change improves performance by avoiding API calls for chart data when the system knows charts won't be functional.
Key Changes
- Early return in TenantDashboard when GraphShard is explicitly false
- Skip chart data fetching in QueriesActivityBar when GraphShard is disabled
- Update comment formatting from JSDoc link to regular markdown link
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| TenantDashboard.tsx | Adds early return when GraphShard is explicitly false and updates comment formatting |
| useQueriesActivityData.ts | Adds GraphShard capability check to skip chart API calls when disabled |
| * If at least one chart successfully loaded, dashboard should be shown | ||
| * This fallback behavior is only used when GraphShardExists capability is not available or false | ||
| * @link https://github.com/ydb-platform/ydb-embedded-ui/issues/659 | ||
| * Link: https://github.com/ydb-platform/ydb-embedded-ui/issues/659 |
Copilot
AI
Sep 2, 2025
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.
The comment format has been changed from JSDoc @link to plain markdown. This breaks JSDoc documentation parsing. Consider reverting to @link https://github.com/ydb-platform/ydb-embedded-ui/issues/659 to maintain proper JSDoc formatting.
| * Link: https://github.com/ydb-platform/ydb-embedded-ui/issues/659 | |
| * @link https://github.com/ydb-platform/ydb-embedded-ui/issues/659 |
Closes #2801
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: ✅
Current: 85.38 MB | Main: 85.38 MB
Diff: +0.95 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information