CONSOLE-5028: Refactor dev-console and knative-plugin Packages to useK8sWatchResource(s)#16011
CONSOLE-5028: Refactor dev-console and knative-plugin Packages to useK8sWatchResource(s)#16011krishagarwal278 wants to merge 3 commits intoopenshift:mainfrom
Conversation
|
@krishagarwal278: This pull request references CONSOLE-5028 which is a valid jira issue. 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. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: krishagarwal278 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest |
krishagarwal278
force-pushed
the
CONSOLE-5028
branch
from
6c1cd49 to
f3f32e1
Compare
…K8sWatchResource(s)
krishagarwal278
force-pushed
the
CONSOLE-5028
branch
from
f3f32e1 to
8f4bd58
Compare
krishagarwal278
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/retest |
|
@krishagarwal278: This pull request references CONSOLE-5028 which is a valid jira issue. 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. |
📝 WalkthroughWalkthroughThis pull request systematically replaces Firehose-based resource provisioning throughout the dev-console and knative-plugin packages with direct Kubernetes watch hooks (useK8sWatchResource and useK8sWatchResources). Affected areas include health checks, deploy image, import forms, monitoring overview, project access, and knative features. Type definitions are standardized to 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/packages/dev-console/src/components/import/ImportSamplePage.tsx (1)
44-46:⚠️ Potential issue | 🟡 MinorHandle
loadErrorto prevent silent failures and improve UX.The hook returns
[data, loaded, loadError], butloadErrorisn't captured. If the ImageStream fails to load (e.g., RBAC denial, resource not found),imageStreamloadedwill betruewhileimageStreamisnull/undefined. This will cause a runtime error on line 52 when accessingbuilderImage.name.Consider destructuring
loadErrorand rendering an appropriate error state:🛡️ Proposed fix
- const [imageStream, imageStreamloaded] = useK8sWatchResource(imageStreamResource); + const [imageStream, imageStreamLoaded, loadError] = useK8sWatchResource(imageStreamResource); - if (!imageStreamloaded) return <LoadingBox />; + if (!imageStreamLoaded) return <LoadingBox />; + if (loadError) { + return ( + <div className="co-m-pane__body"> + <div className="pf-v6-u-text-align-center"> + {t('devconsole~Error loading ImageStream: {{error}}', { error: loadError.message })} + </div> + </div> + ); + }Also consider using a shared error component pattern if one exists in the codebase (e.g.,
StatusBoxwithloadError).frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModalController.tsx (1)
175-183:⚠️ Potential issue | 🟠 MajorMissing
returnfor promise chain - errors from delete won't propagate.On line 177,
deleteRevisionAction(action)returns a Promise, but it's not being returned from the.then()callback. Ifk8sPatchsucceeds butk8sKillfails, the error won't propagate to the.catch()block, leaving the user unaware of the failure and potentially in an inconsistent state (traffic redistributed, but revision not deleted).🐛 Fix: return the promise to maintain the chain
return k8sPatch(ServiceModel, service, ksvcPatch) .then(() => { - deleteRevisionAction(action); + return deleteRevisionAction(action); }) .catch((err) => {frontend/packages/dev-console/src/components/import/ImportForm.tsx (1)
168-168:⚠️ Potential issue | 🟡 MinorPotential undefined access on
projects.The
projectsprop is typed as optional (projects?at line 62), but line 168 accessesprojects.loadedwithout a guard. IfImportFormis ever rendered without theprojectsprop, this will throw a runtime error.Consider adding a defensive check:
🛡️ Proposed fix
- const createNewProject = projects.loaded && _.isEmpty(projects.data); + const createNewProject = projects?.loaded && _.isEmpty(projects.data);frontend/packages/dev-console/src/components/project-access/ProjectAccess.tsx (1)
33-53:⚠️ Potential issue | 🟠 MajorGuard optional
roleBindingsand use explicit error detection.
roleBindingsis optional in the interface but dereferenced immediately on line 47 without null checks, causing a crash if the prop is omitted. Additionally,_.isEmpty(new Error())returnstrue(Error objects have no enumerable keys), so the current guard masks error states and incorrectly showsLoadingBoxwhen an error exists. Update the destructuring to provide a default value and replace_.isEmpty()with an explicitinstanceof Errorcheck.🛠️ Proposed fix
const ProjectAccess: FC<ProjectAccessProps> = ({ namespace, - roleBindings, + roleBindings = { data: [], loaded: false, loadError: undefined }, roles, fullFormView, }) => { const { t } = useTranslation(); - if ((!roleBindings.loaded && _.isEmpty(roleBindings.loadError)) || !roles.loaded) { + const hasLoadError = + roleBindings.loadError instanceof Error || !_.isEmpty(roleBindings.loadError); + if ((!roleBindings.loaded && !hasLoadError) || !roles.loaded) { return <LoadingBox />; }
🤖 Fix all issues with AI agents
In
`@frontend/packages/dev-console/src/components/health-checks/HealthChecksPage.tsx`:
- Around line 10-15: The route params (kind, ns, name) from useParams may be
undefined, so change HealthChecksPage.tsx to avoid calling useK8sWatchResource
with undefined values: either add an early guard in the component that returns a
loading/error UI when any of kind/ns/name is missing, or switch to a conditional
watch by only invoking useK8sWatchResource (or passing null as the resource
argument) when all three params are defined; reference the existing
useK8sWatchResource call and the kind/ns/name variables to implement the guard
or conditional call.
In
`@frontend/packages/knative-plugin/src/components/knatify/CreateKnatifyPage.tsx`:
- Around line 140-143: The form is being rendered with
projects={resources?.projects as WatchK8sResultsObject<...>} which can be
undefined; update the rendering guard in CreateKnatifyPage to ensure
resources.projects exists and is loaded before rendering KnatifyForm (or pass a
safe default typed WatchK8sResultsObject) — e.g., check resources?.projects &&
isResourceLoaded(resources.projects) (or similar) using the existing
isResourceLoaded helper and only pass formikProps to KnatifyForm when that
condition is true so KnatifyForm never receives an undefined projects prop.
In
`@frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModalController.tsx`:
- Around line 75-81: The current loaded check in
DeleteRevisionModalController.tsx only ensures resources[key].loaded but ignores
resources[key].loadError; update the readiness gating to also detect loadError
(e.g., treat a resource as not-ok if resources[key].loadError is truthy) and
specifically guard the non-optional services resource: if services.loadError (or
any resource.loadError) is present, return or render an error state/modal
instead of proceeding with deletion logic; ensure any early-return that
currently uses the loaded variable is updated to incorporate the new error
condition so stale/failed watches don’t allow the component to continue.
In
`@frontend/packages/knative-plugin/src/components/test-function/TestFunctionController.tsx`:
- Around line 19-26: Update TestFunctionController to destructure the third
return value from useK8sWatchResource (loadError) and handle three states: show
a brief loading indicator while not loaded, show an inline error (or call
props.closeOverlay with a notification) when loadError is present, and guard
against service being undefined before rendering <TestFunction {...props}
service={service} />; reference the useK8sWatchResource call in
TestFunctionController and the TestFunction component to add the error check and
the fallback UI for loading and missing service.
In
`@frontend/packages/knative-plugin/src/components/traffic-splitting/TrafficSplittingController.tsx`:
- Around line 48-50: The controller currently returns null when resources are
still loading; update TrafficSplittingController so that when loaded is false it
renders the project's LoadingBox component instead of null (i.e., return
<LoadingBox />), keeping the existing return when loaded is true:
<TrafficSplitting {...props} service={props.obj} revisions={revisions} />; also
add an import for LoadingBox if not already present and ensure
getKnativeRevisionsData and TrafficSplitting usage remain unchanged.
🧹 Nitpick comments (8)
frontend/packages/dev-console/src/components/import/ImportSamplePage.tsx (1)
33-42: Remove Firehose-specificpropfield from resource definition.The
prop: 'imageStreams'field is a Firehose artifact used to name the injected prop.useK8sWatchResourcedoesn't utilize this field — it simply returns the resource data directly. Leaving it in creates confusion about what's actually used.🧹 Proposed cleanup
const imageStreamResource = useMemo( () => ({ kind: ImageStreamModel.kind, - prop: 'imageStreams', isList: false, name: imageStreamName, namespace: imageStreamNamespace, }), [imageStreamName, imageStreamNamespace], );frontend/packages/knative-plugin/src/components/test-function/TestFunctionController.tsx (1)
39-42: Stable reference forpropsinuseCallbackdependency.If
propsis a new object reference on each render (common pattern when spreading inline), the memoized callback will be recreated every time, defeating the purpose ofuseCallback. Verify that the caller passes a stablepropsobject or consider destructuring only the required properties into the dependency array.frontend/packages/dev-console/src/components/health-checks/HealthChecksPage.tsx (1)
17-21: Consider memoizing the resource object.This object is recreated on every render. If
AddHealthChecksFormor its descendants perform reference equality checks (e.g., inuseEffectdependencies orReact.memo), this could trigger unnecessary re-renders.♻️ Proposed refactor using useMemo
+import { useMemo } from 'react';- const resource = { - data: resourceData, - loaded, - loadError, - }; + const resource = useMemo( + () => ({ data: resourceData, loaded, loadError }), + [resourceData, loaded, loadError], + );frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModalController.tsx (1)
153-167: ConsideruseCallbackfordeleteRevisionAction(optional).The function is recreated on every render. For a modal component with user-triggered actions, this is acceptable, but wrapping in
useCallbackwith[revision, close, t]dependencies would be a minor optimization if this pattern is used elsewhere.The redirect logic using regex to check if we're on the deleted revision's page is a solid pattern for avoiding broken views post-deletion.
frontend/packages/knative-plugin/src/components/traffic-splitting/TrafficSplittingController.tsx (1)
44-46: Consider simplifying loaded check and handling loadError.The
Object.keys(resources).length > 0guard is redundant sincewatchResourcesalways defines bothrevisionsandconfigurationskeys—the object will never be empty. More importantly, whenloadErroris present on either resource, the user currently sees nothing (null render) with no feedback.Since resources are
optional: true, errors may be expected, but for a traffic splitting modal, consider surfacing errors or at least logging them for debuggability.♻️ Suggested simplification with error awareness
- const loaded = - Object.keys(resources).length > 0 && - Object.keys(resources).every((key) => resources[key].loaded); + const loaded = resources.revisions.loaded && resources.configurations.loaded; + const loadError = resources.revisions.loadError || resources.configurations.loadError; + + if (loadError) { + // eslint-disable-next-line no-console + console.warn('TrafficSplittingController: Failed to load resources', loadError); + }frontend/packages/dev-console/src/components/import/DeployImagePage.tsx (1)
19-22: Consider usingProjectModel.kindinstead of string literal.For consistency with
ImportPage.tsx(which usesProjectModel.kindat line 67), consider importing and usingProjectModelhere. This ensures the kind string stays in sync if the model definition ever changes.♻️ Suggested refactor
+import { ProjectModel } from '@console/internal/models'; import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook'; import { K8sResourceKind } from '@console/internal/module/k8s';const [projectsData, loaded, loadError] = useK8sWatchResource<K8sResourceKind[]>({ - kind: 'Project', + kind: ProjectModel.kind, isList: true, });frontend/packages/dev-console/src/components/import/ImportPage.tsx (1)
49-50: Edge case handling is graceful but lacks test coverage.The code correctly guards against partial
imagestreamparameters by requiring bothimageStreamNameANDimageStreamNamespace—if either is missing, it silently falls back to git import mode. Analysis of the codebase shows the only identified URL construction path inuseBuilderImages.tsxalways provides both parameters together, so this edge case shouldn't occur in normal operation. However, this fallback behavior is neither documented nor tested, and users navigating with malformed URLs would silently get git import rather than an error. Consider adding a test case covering incomplete URL parameters to prevent future regressions.frontend/packages/dev-console/src/components/monitoring/overview/MonitoringTab.tsx (1)
48-69: Preserve pod event loadError for parity.
Right now per-pod watch errors are dropped; carryingloadErrorkeeps the shape consistent and makes future error handling easier.🔧 Suggested update
- const podEventProps: Record<string, { data: EventKind[]; loaded: boolean }> = {}; + const podEventProps: Record<string, { data: EventKind[]; loaded: boolean; loadError?: Error }> = {}; if (podData?.pods) { podData.pods.forEach((pod) => { const podEvents = resources[pod.metadata.uid]; if (podEvents) { podEventProps[pod.metadata.uid] = { data: podEvents.data || [], loaded: podEvents.loaded, + loadError: podEvents.loadError, }; } }); }
| const [resourceData, loaded, loadError] = useK8sWatchResource<K8sResourceKind>({ | ||
| kind, | ||
| namespace: ns, | ||
| isList: false, | ||
| name, | ||
| }); |
There was a problem hiding this comment.
Route params may be undefined — consider guard or conditional watch.
The kind, ns, and name values from useParams() are typed as string | undefined. If any of these are missing (e.g., malformed URL or route mismatch), useK8sWatchResource will attempt to watch with undefined values, which could lead to unexpected API calls or silent failures.
Consider either:
- Adding a guard clause that returns early (e.g., a loading state or error) when required params are missing, or
- Using a conditional resource watch pattern (passing
nullto skip the watch when params are invalid).
🛡️ Proposed fix using conditional watch
- const [resourceData, loaded, loadError] = useK8sWatchResource<K8sResourceKind>({
- kind,
- namespace: ns,
- isList: false,
- name,
- });
+ const watchResource = kind && ns && name
+ ? { kind, namespace: ns, isList: false, name }
+ : null;
+
+ const [resourceData, loaded, loadError] = useK8sWatchResource<K8sResourceKind>(watchResource);🤖 Prompt for AI Agents
In
`@frontend/packages/dev-console/src/components/health-checks/HealthChecksPage.tsx`
around lines 10 - 15, The route params (kind, ns, name) from useParams may be
undefined, so change HealthChecksPage.tsx to avoid calling useK8sWatchResource
with undefined values: either add an early guard in the component that returns a
loading/error UI when any of kind/ns/name is missing, or switch to a conditional
watch by only invoking useK8sWatchResource (or passing null as the resource
argument) when all three params are defined; reference the existing
useK8sWatchResource call and the kind/ns/name variables to implement the guard
or conditional call.
| <KnatifyForm | ||
| {...formikProps} | ||
| projects={(resources?.projects as WatchK8sResultsObject<K8sResourceKind[]>) ?? {}} | ||
| projects={resources?.projects as WatchK8sResultsObject<K8sResourceKind[]>} | ||
| /> |
There was a problem hiding this comment.
Guard against undefined projects before rendering the form.
projects={resources?.projects as ...} can still be undefined while isResourceLoaded is true (since the current check doesn’t require the key). That’s a latent runtime risk if KnatifyForm dereferences projects.loaded/data. Consider tightening the loaded check to require resources.projects (or providing a typed default) before rendering.
✅ Tighten the loaded gate to require projects
- const isResourceLoaded =
- Object.keys(resources).length > 0 &&
- Object.values(resources).every((value) => value.loaded || !!value.loadError) &&
- (hpaLoaded || !!hpaError);
+ const isResourceLoaded =
+ !!resources.projects &&
+ Object.values(resources).every((value) => value.loaded || !!value.loadError) &&
+ (hpaLoaded || !!hpaError);🤖 Prompt for AI Agents
In
`@frontend/packages/knative-plugin/src/components/knatify/CreateKnatifyPage.tsx`
around lines 140 - 143, The form is being rendered with
projects={resources?.projects as WatchK8sResultsObject<...>} which can be
undefined; update the rendering guard in CreateKnatifyPage to ensure
resources.projects exists and is loaded before rendering KnatifyForm (or pass a
safe default typed WatchK8sResultsObject) — e.g., check resources?.projects &&
isResourceLoaded(resources.projects) (or similar) using the existing
isResourceLoaded helper and only pass formikProps to KnatifyForm when that
condition is true so KnatifyForm never receives an undefined projects prop.
| const loaded = | ||
| Object.keys(resources).length > 0 && | ||
| Object.keys(resources).every((key) => resources[key].loaded); | ||
|
|
||
| if (!loaded) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Missing loadError handling for watched resources.
The loaded check only verifies that resources have finished loading but doesn't account for loadError. If the services watch fails (e.g., RBAC denial), the component will proceed with potentially empty or stale data, leading to confusing UX.
Consider checking for errors, especially on the non-optional services resource:
🛡️ Suggested defensive error handling
const loaded =
Object.keys(resources).length > 0 &&
Object.keys(resources).every((key) => resources[key].loaded);
+ const loadError = resources.services?.loadError;
+
+ if (loadError) {
+ // Consider rendering an error state or logging
+ console.error('Failed to load services:', loadError);
+ }
+
if (!loaded) {
return null;
}Alternatively, you could display a user-facing error modal if the required service data can't be loaded.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const loaded = | |
| Object.keys(resources).length > 0 && | |
| Object.keys(resources).every((key) => resources[key].loaded); | |
| if (!loaded) { | |
| return null; | |
| } | |
| const loaded = | |
| Object.keys(resources).length > 0 && | |
| Object.keys(resources).every((key) => resources[key].loaded); | |
| const loadError = resources.services?.loadError; | |
| if (loadError) { | |
| // Consider rendering an error state or logging | |
| console.error('Failed to load services:', loadError); | |
| } | |
| if (!loaded) { | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In
`@frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModalController.tsx`
around lines 75 - 81, The current loaded check in
DeleteRevisionModalController.tsx only ensures resources[key].loaded but ignores
resources[key].loadError; update the readiness gating to also detect loadError
(e.g., treat a resource as not-ok if resources[key].loadError is truthy) and
specifically guard the non-optional services resource: if services.loadError (or
any resource.loadError) is present, return or render an error state/modal
instead of proceeding with deletion logic; ensure any early-return that
currently uses the loaded variable is updated to incorporate the new error
condition so stale/failed watches don’t allow the component to continue.
| const [service, loaded] = useK8sWatchResource<ServiceKind>({ | ||
| kind: referenceForModel(ServiceModel), | ||
| isList: false, | ||
| namespace: obj.metadata.namespace, | ||
| name: obj.metadata.name, | ||
| }); | ||
|
|
||
| return ( | ||
| <Firehose resources={serverlessResources}> | ||
| <Controller {...props} /> | ||
| </Firehose> | ||
| ); | ||
| return loaded ? <TestFunction {...props} service={service} /> : null; |
There was a problem hiding this comment.
Handle loadError and guard against undefined service.
The useK8sWatchResource hook returns a tuple of [data, loaded, loadError]. Currently, loadError is not destructured, so users receive no feedback if the resource fetch fails—the component just renders null indefinitely once loaded becomes true with an error state. Additionally, service may be undefined even when loaded is true (e.g., resource not found), which could cause issues downstream in TestFunction.
Consider also providing a brief loading indicator rather than null to avoid a flash of empty content in the modal.
🛡️ Proposed fix to handle error state and guard service
- const [service, loaded] = useK8sWatchResource<ServiceKind>({
+ const [service, loaded, loadError] = useK8sWatchResource<ServiceKind>({
kind: referenceForModel(ServiceModel),
isList: false,
namespace: obj.metadata.namespace,
name: obj.metadata.name,
});
- return loaded ? <TestFunction {...props} service={service} /> : null;
+ if (!loaded) {
+ return null; // Consider a <LoadingBox /> here for better UX
+ }
+
+ if (loadError || !service) {
+ // Optionally render an error state or close the modal
+ return null;
+ }
+
+ return <TestFunction {...props} service={service} />;For a production-quality modal, you may want to show an inline error message (e.g., using PatternFly's Alert component) or call props.closeOverlay with an appropriate notification when the resource cannot be loaded.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [service, loaded] = useK8sWatchResource<ServiceKind>({ | |
| kind: referenceForModel(ServiceModel), | |
| isList: false, | |
| namespace: obj.metadata.namespace, | |
| name: obj.metadata.name, | |
| }); | |
| return ( | |
| <Firehose resources={serverlessResources}> | |
| <Controller {...props} /> | |
| </Firehose> | |
| ); | |
| return loaded ? <TestFunction {...props} service={service} /> : null; | |
| const [service, loaded, loadError] = useK8sWatchResource<ServiceKind>({ | |
| kind: referenceForModel(ServiceModel), | |
| isList: false, | |
| namespace: obj.metadata.namespace, | |
| name: obj.metadata.name, | |
| }); | |
| if (!loaded) { | |
| return null; // Consider a <LoadingBox /> here for better UX | |
| } | |
| if (loadError || !service) { | |
| // Optionally render an error state or close the modal | |
| return null; | |
| } | |
| return <TestFunction {...props} service={service} />; |
🤖 Prompt for AI Agents
In
`@frontend/packages/knative-plugin/src/components/test-function/TestFunctionController.tsx`
around lines 19 - 26, Update TestFunctionController to destructure the third
return value from useK8sWatchResource (loadError) and handle three states: show
a brief loading indicator while not loaded, show an inline error (or call
props.closeOverlay with a notification) when loadError is present, and guard
against service being undefined before rendering <TestFunction {...props}
service={service} />; reference the useK8sWatchResource call in
TestFunctionController and the TestFunction component to add the error check and
the fallback UI for loading and missing service.
| const revisions = getKnativeRevisionsData(props.obj, resources); | ||
|
|
||
| return loaded ? <TrafficSplitting {...props} service={props.obj} revisions={revisions} /> : null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the knative-topology-utils file
find frontend -name "*knative*topology*utils*" -type fRepository: openshift/console
Length of output: 219
🏁 Script executed:
#!/bin/bash
# Search for getKnativeRevisionsData definition
rg -n 'getKnativeRevisionsData' frontend --type=ts -A 5 -B 2Repository: openshift/console
Length of output: 6348
🏁 Script executed:
#!/bin/bash
# Find useK8sWatchResources to understand the structure it returns
rg -n 'useK8sWatchResources' frontend/packages/knative-plugin --type=ts -A 3 -B 2Repository: openshift/console
Length of output: 30355
🏁 Script executed:
#!/bin/bash
# Check for LoadingBox usage patterns in the knative-plugin
rg -n 'LoadingBox' frontend/packages/knative-plugin --type=tsRepository: openshift/console
Length of output: 1961
🏁 Script executed:
#!/bin/bash
# Get the full getKnativeRevisionsData function implementation
sed -n '495,520p' frontend/packages/knative-plugin/src/topology/knative-topology-utils.tsRepository: openshift/console
Length of output: 1014
🏁 Script executed:
#!/bin/bash
# Check TopologyDataResources type definition
rg -n 'type TopologyDataResources' frontend/packages/knative-plugin --type=ts -A 5Repository: openshift/console
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Verify the exact structure returned by useK8sWatchResources in the controller
sed -n '35,52p' frontend/packages/knative-plugin/src/components/traffic-splitting/TrafficSplittingController.tsxRepository: openshift/console
Length of output: 527
🏁 Script executed:
#!/bin/bash
# Find TopologyDataResources type definition in the utils file
rg -n 'TopologyDataResources' frontend/packages/knative-plugin/src/topology/knative-topology-utils.ts -B 2 -A 8 | head -30Repository: openshift/console
Length of output: 1165
🏁 Script executed:
#!/bin/bash
# Look for TopologyDataResources definition in the imported console topology package
# First, check if it's documented in any local type files or interfaces
rg -n 'interface TopologyDataResources\|type TopologyDataResources' frontend --type=tsRepository: openshift/console
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Verify the pattern across other usages in DeleteRevisionModalController to confirm consistency
sed -n '65,95p' frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModalController.tsxRepository: openshift/console
Length of output: 907
Use LoadingBox for visual feedback during resource loading.
The resources structure from useK8sWatchResources correctly resolves via optional chaining—resources.configurations?.data and resources.revisions?.data align properly with the hook's return shape. However, returning null during the loading state leaves the modal with no visual feedback. The codebase consistently uses <LoadingBox /> for this pattern (e.g., CreateKnatifyPage, EventSourcePage, Subscribe). Replace the null return with a loading spinner for better perceived responsiveness.
🤖 Prompt for AI Agents
In
`@frontend/packages/knative-plugin/src/components/traffic-splitting/TrafficSplittingController.tsx`
around lines 48 - 50, The controller currently returns null when resources are
still loading; update TrafficSplittingController so that when loaded is false it
renders the project's LoadingBox component instead of null (i.e., return
<LoadingBox />), keeping the existing return when loaded is true:
<TrafficSplitting {...props} service={props.obj} revisions={revisions} />; also
add an import for LoadingBox if not already present and ensure
getKnativeRevisionsData and TrafficSplitting usage remain unchanged.
3eb4274 to
ff0ebb0
Compare
|
/retest |
|
@krishagarwal278: This pull request references CONSOLE-5028 which is a valid jira issue. 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. |
|
@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. |
3 participants
Tested locally to check refactored components in Dev-Console and knative-plugin packages are working as expected:
Summary by CodeRabbit
Release Notes