-
Notifications
You must be signed in to change notification settings - Fork 667
OCPBUGS-63120: Home -> Overview page returns error during upgrade process #15831
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
OCPBUGS-63120: Home -> Overview page returns error during upgrade process #15831
Conversation
|
@krishagarwal278: This pull request references Jira Issue OCPBUGS-63120, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughAdds an early guard in the TopologyPage component to return null when Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/packages/topology/src/components/page/TopologyPage.tsx (1)
132-134: Clarify whenviewTypecan be falsy; current guard may be redundant or mask the root causeGiven how
viewTypeis derived:const viewType = (queryParams.get('view') as TopologyViewType) || topologyViewState || defaultViewType;and that
defaultViewTypedefaults toTopologyViewType.graphin the props destructuring,viewTypeshould be truthy under the current type contract. If you’re actually seeing a falsyviewTypeduring upgrade, that suggests an upstream issue (e.g., invalid/undefined value coming from user settings, query params, or a caller passing an unexpected value) that isn’t reflected in the types.Returning
nullhere will silently render a blank page and may hide that underlying problem.It would be good to either:
- Document the specific scenario where
viewTypeis falsy (so future readers understand why this guard exists), or- Tighten the fix closer to the source (e.g., normalizing
topologyViewState/defaultViewTypeor gating onloaded) soviewTypeis guaranteed non-falsy and this guard isn’t needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
frontend/packages/topology/src/components/page/TopologyPage.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/topology/src/components/page/TopologyPage.tsx
|
/retest |
3cf0d0c to
c15608b
Compare
|
Launched a 4.20 cluster, upgraded the cluster to image built from the pr code. During upgrade, kept observing the Home->Overview page, there was not error on the page. The upgrade finished without error. /verified by yanpzhan |
|
@yanpzhan: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@krishagarwal278: This pull request references Jira Issue OCPBUGS-63120, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
1 similar comment
|
/retest |
|
/jira refresh |
|
@vikram-raj: This pull request references Jira Issue OCPBUGS-63120, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
vikram-raj
left a comment
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.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: krishagarwal278, vikram-raj The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/acknowledge-critical-fixes-only |
|
/label acknowledge-critical-fixes-only |
|
/retest |
|
/hold Revision c15608b was retested 3 times: holding |
|
/unhold |
|
/retest |
|
/jira refresh The requirements for Jira bugs have changed (Jira issues linked to PRs on main branch need to target different OCP), recalculating validity. |
|
@openshift-bot: This pull request references Jira Issue OCPBUGS-63120, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/hold Revision c15608b was retested 3 times: holding |
|
/unhold |
|
@krishagarwal278: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@krishagarwal278: Jira Issue Verification Checks: Jira Issue OCPBUGS-63120 Jira Issue OCPBUGS-63120 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
No description provided.