fix: use healthcheck self_check_result for database status color#3837
fix: use healthcheck self_check_result for database status color#3837
Conversation
|
@copilot no, you shouldn't add a new mapping from healthcheck status to color. it's already exists in the project, look for it. |
Removed the new |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the database status indicator on the Tenant diagnostics overview to derive its color from the healthcheck database_status signal (when available), aligning it with the healthcheck drawer/preview.
Changes:
- Subscribe to
healthcheckApi.useGetHealthcheckInfoQueryinTenantOverview(skipped for Serverless). - Map
healthcheckData.database_status[0].overall(StatusFlag) toEntityStatus’s expectedEFlagusinghcStatusToColorFlag. - Fall back to
tenantinfo.Overallwhen healthcheck data isn’t available.
|
@copilot review the comments and fix them. also fix the PR title |
Addressed all reviewer comments in 0348cac:
PR title updated to "Use healthcheck database_status for database status color". |
|
@copilot here is example of /viewer/healthcheck response: you should use "self_check_result" field to convert it's values to colors and present it as the database status |
... Switched to
Falls back to |
@copilot omg, why you've added another mapping from hc status to flag AGAIN?! self_check_result IS a hcStatus. health check status = self_check_result. use existing mapper, it should be enough |
... Done in 1baecbf. Removed the separate |
okay, this is not what I meant. there is should be existing convertor from "EMERGENCY" to "RED". could you please search codebase carefully and find that convertor. and use it instead. |
... I searched the codebase carefully and can't find an existing
Could you point me at the specific file/symbol you have in mind? Or did you mean: instead of |
|
@copilot Use the existing Please do not extend it with self_check_result -> StatusFlag -> hcStatusToColorFlag -> EFlagThe mapping should match the backend logic in GOOD -> GREEN
DEGRADED -> YELLOW
MAINTENANCE_REQUIRED -> RED
EMERGENCY -> RED
UNSPECIFIED/default -> GREYSo the fix should be:
Do not map |
Agent-Logs-Url: https://github.com/ydb-platform/ydb-embedded-ui/sessions/0e3a347f-ddf8-40d8-bfae-c0fc429cd540 Co-authored-by: astandrik <8037318+astandrik@users.noreply.github.com>
Updated in 92c4050.
Screenshot: |
On the database page the status indicator next to the database name was driven by
tenantinfo.Overall. Switch it to the healthcheck response so the color reflects the same signal surfaced by the healthcheck drawer/preview, while preserving the existingStatusFlag→EFlagcolor mapper already present in the project.Changes
TenantOverview— subscribes tohealthcheckApi.useGetHealthcheckInfoQueryand readshealthcheckData.self_check_result. Before passing the value toEntityStatus, it now convertsSelfCheckResulttoStatusFlag, then reuses the existinghcStatusToColorFlagmapper. Falls back totenant.Overallwhen healthcheck data is absent (Serverless, loading, or error), so behavior is unchanged in those cases.selfCheckResultToHcStatus— added instore/reducers/healthcheckInfo/utils.tsto normalizeSelfCheckResulttoStatusFlagusing backend-compatible semantics:GOOD → GREENDEGRADED → YELLOWMAINTENANCE_REQUIRED → REDEMERGENCY → REDUNSPECIFIED → GREYhcStatusToColorFlag— kept asRecord<StatusFlag, EFlag>only; it is no longer extended withSelfCheckResultkeys.HealthcheckPreviewsubscription, which shares the same RTK Query cache key, so this subscription does not alter polling frequency or load.?? Overallensures that a missing mapping key cleanly falls back totenant.Overallrather than passingundefinedtoEntityStatus.Important Files Changed
self_check_resultthroughSelfCheckResult -> StatusFlag -> EFlagbefore renderingEntityStatus; falls back totenant.Overallwhen data is absent.hcStatusToColorFlagscoped toStatusFlagonly and addsselfCheckResultToHcStatusto normalize healthcheckself_check_resultvalues to backend-compatibleStatusFlagvalues.Greptile Summary
The PR aligns the database status badge color in
TenantOverviewwith the healthcheck drawer by readinghealthcheckData?.self_check_resultinstead oftenant.Overall. It now normalizesSelfCheckResulttoStatusFlagfirst, then reuses the existinghcStatusToColorFlagmapping, preserving the original mapper responsibilities and matching backend cluster-state semantics. Skip conditions (isServerless,isV2NavigationEnabled,tenant === undefined) are correctly applied, and RTK Query deduplicates this subscription againstHealthcheckPreview's polling subscription so no extra network requests are introduced.Confidence Score: 5/5
Safe to merge — logic is correct, fallback is solid, skip conditions are properly guarded, and the final status semantics now match backend behavior.
No P0 or P1 issues found. The RTK Query cache deduplication with
HealthcheckPreviewworks as intended, and the final implementation avoids duplicatingSelfCheckResult -> EFlagcolor mapping logic.No files require special attention.
Important Files Changed
self_check_resultviaStatusFlagwith safe fallback toOverallhcStatusToColorFlagtoStatusFlag -> EFlagonly and addsselfCheckResultToHcStatusfor backend-compatible normalizationFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[TenantOverview mounts] --> B{skip conditions} B -->|isServerless OR\nisV2NavigationEnabled OR\ntenant === undefined| C[Skip healthcheck query\nfallback: databaseStatus = Overall] B -->|none| D[useGetHealthcheckInfoQuery\ndatabase] D --> E{RTK Query cache hit?} E -->|yes - HealthcheckPreview\nshares same key| F[Return cached data\nno extra network req] E -->|no| G[Network request] G --> F F --> H{self_check_result\ndefined?} H -->|undefined| C H -->|defined| I[selfCheckResultToHcStatus\nSelfCheckResult to StatusFlag] I --> J[hcStatusToColorFlag\nStatusFlag to EFlag] J --> K{mapping found?} K -->|yes| L[databaseStatus = EFlag] K -->|no, fallback via ??| C C --> M[EntityStatus with databaseStatus] L --> MReviews (5): Last reviewed commit: "fix: convert self check result through status flag" | Re-trigger Greptile
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
😟 No changes in tests. 😕
Bundle Size: ✅
Current: 63.46 MB | Main: 63.46 MB
Diff: +2.35 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information