Topology, fetch build config data when necessary #7162
Conversation
openshift-ci-robot
added
kind/cleanup
openshift-ci-robot
added
component/dev-console
jeff-phillips-18
force-pushed
the
topology-load-build-configs
branch
from
b5cfe8e
to
fb36344
Compare
| const errorKey = Object.keys(resources).find((key) => resources[key].loadError); | ||
| if (errorKey) { | ||
| setLoadError(resources[errorKey].loadError); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Small detail, but can simplified with Object.values:
| const errorKey = Object.keys(resources).find((key) => resources[key].loadError); | |
| if (errorKey) { | |
| setLoadError(resources[errorKey].loadError); | |
| return; | |
| } | |
| const resourceWithLoadError = Object.values(resources).find((r) => r.loadError); | |
| if (resourceWithLoadError) { | |
| setLoadError(resourceWithLoadError.loadError); | |
| return; | |
| } |
For me both is fine, doesn't block this PR for me.
| resource: K8sResourceKind, | ||
| ): { loaded: boolean; loadError: string; buildConfigs: BuildConfigOverviewItem[] } => { | ||
| const [loaded, setLoaded] = React.useState<boolean>(false); | ||
| const [loadError, setLoadError] = React.useState<string>(''); |
There was a problem hiding this comment.
Please default with null. The useEffect also sets setLoadError to null if there is new resources available without any error. In this case we can save us from one re-rendering
| const resourceBuildConfigs = | ||
| resource.kind === 'CronJob' | ||
| ? getBuildConfigsForCronJob(resource as CronJobKind, resources) | ||
| : getBuildConfigsForResource(resource, resources); |
There was a problem hiding this comment.
Just an idea: We can move this check also into getBuildConfigsForResource ? So that it starts with
export const getBuildConfigsForResource = (
resource: K8sResourceKind,
resources: any,
): BuildConfigOverviewItem[] => {
if (resource.kind === 'CronJob') {
return getBuildConfigsForCronJob(resource as CronJobKind, resources);
}
// ....Makes this sense? Not required to get a lgtm :)
jeff-phillips-18
force-pushed
the
topology-load-build-configs
branch
from
fb36344
to
e611720
Compare
jeff-phillips-18
force-pushed
the
topology-load-build-configs
branch
from
e611720
to
705eb7e
Compare
|
/test kubevirt-plugin |
|
/test kubevirt-plugin |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeff-phillips-18, jerolimov, rohitkrai03 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
Fixes:
https://issues.redhat.com/browse/ODC-5114
Solution Description:
Remove build configs from OverviewItem type. Add hook to retrieve build configs for a given resource. Update instances to use hooks rather than fields in the OverviewItem type.
The alerts field for OverviewItem type is also removed as it is not being used anywhere (but required loading buildConfig resources).
Non-Functional Change
Browser conformance:
/kind cleanup