feat(Diagnostics): split CDC and its implementation#2281
Merged
artemmufazalov merged 1 commit intomainfrom May 19, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the diagnostics functionality by splitting CDC and its implementation, aiming to fix the Tablets tab bug and simplify the underlying logic. Key changes include removing multi-overview queries and related selectors, updating schema mappings for CDC stream types, and adding a new subType prop that cascades through various diagnostic components.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/store/reducers/overview/overview.ts | Removed multi-overview query and selectors for cleanup. |
| src/containers/Tenant/utils/schema.ts | Updated schema mappings; removed isEntityWithMergedImplementation. |
| src/containers/Tenant/Tenant.tsx | Added subType prop to pass along to ObjectGeneral. |
| src/containers/Tenant/ObjectGeneral/ObjectGeneral.tsx | Propagated subType to Diagnostics component. |
| src/containers/Tenant/Diagnostics/Overview/Overview.tsx | Refactored CDC stream handling using currentData from API. |
| src/containers/Tenant/Diagnostics/Overview/ChangefeedInfo/ChangefeedInfo.tsx | Removed topic parameter and adjusted ChangefeedInfo behavior. |
| src/containers/Tenant/Diagnostics/DiagnosticsPages.ts | Introduced new CDC_STREAM_IMPL_PAGES mapping. |
| src/containers/Tenant/Diagnostics/Diagnostics.ts | Updated diagnostics routing by including subType. |
| src/containers/Tenant/Diagnostics/Describe/Describe.tsx | Simplified Describe component by removing type-dependent logic. |
Comments suppressed due to low confidence (2)
src/containers/Tenant/utils/schema.ts:226
- The change in mapping for EPathType.EPathTypeCdcStream from true to false alters the behavior of CDC stream handling. Please confirm that this change is intentional and aligns with the updated CDC implementation requirements.
[EPathType.EPathTypeCdcStream]: false,
src/containers/Tenant/Diagnostics/Overview/ChangefeedInfo/ChangefeedInfo.tsx:45
- Removing the topic parameter in ChangefeedInfo changes its behavior by no longer requiring separate topic data. Verify that this refactoring satisfies the intended CDC stream diagnostics and that no additional topic-related logic is needed.
if (!data) {
Raubzeug
approved these changes
May 19, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2142
Stand: https://nda.ya.ru/t/GHkj9rbC7ETqAw
Motivation: fix current bugs (not working Tablets tab) and prevent further bugs connected with complex logic of merging entity and its implementation
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
😟 No changes in tests. 😕
Bundle Size: 🔽
Current: 83.52 MB | Main: 83.53 MB
Diff: 0.01 MB (-0.01%)
✅ Bundle size decreased.
ℹ️ CI Information