Bug 1911269: hide "waiting for builds" info message when a build is present #7665

debsmita1 · 2020-12-28T12:42:07Z

Fixes:
https://issues.redhat.com/browse/ODC-5069

Analysis / Root cause:
It is not being checked if a successful build exists

Solution Description:

Checking if there exist some pod(s) having at least one(or more) container(s) with image id
added translation

GIF:

Browser conformance:

Chrome
Firefox
Safari
Edge

debsmita1 · 2020-12-28T12:42:45Z

/kind bug

openshift-ci-robot · 2020-12-28T12:47:27Z

@debsmita1: This pull request references Bugzilla bug 1911269, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1911269: hide "waiting for builds" info message when a build is present

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 kubernetes/test-infra repository.

debsmita1 · 2020-12-28T12:47:41Z

/cc @jerolimov

divyanshiGupta · 2021-01-04T12:55:41Z

frontend/public/components/overview/pods-overview.tsx

 }) => {
  const {
    metadata: { name, namespace },
  } = obj;

  const [showWaitingPods, setShowWaitingPods] = React.useState(false);
  const showWaitingForBuildAlert =
-    hasBuildConfig && isDeploymentGeneratedByWebConsole(obj) && pods.some(isPodWithoutImageId);
+    !isBuildComplete && isDeploymentGeneratedByWebConsole(obj) && pods.some(isPodWithoutImageId);


This doesn't seem like the right solution as when the build completes this pods.some(isPodWithoutImageId) should return false. I think you should look into fixing this check and see why this isn't returning false when the build completes.

openshift-ci-robot · 2021-01-04T14:05:56Z

@debsmita1: This pull request references Bugzilla bug 1911269, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1911269: hide "waiting for builds" info message when a build is present

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 kubernetes/test-infra repository.

jerolimov · 2021-01-05T11:02:22Z

/retest

rohitkrai03 · 2021-01-07T13:44:18Z

frontend/public/components/overview/pods-overview.tsx

@@ -68,6 +68,9 @@ const isPodWithoutImageId = (pod: PodKind) =>
  pod.status?.phase === 'Pending' &&
  pod.status?.containerStatuses?.some((containerStatus) => !containerStatus.imageID);

+const isPodWithImageId = (pod: PodKind) =>


Why do we need two different checks here isPodWithoutImageId and isPodWithImageId ? Can we not use just one check for both cases?

^^ @debsmita1

removed the usage of isPodWithoutImageId

openshift-ci-robot · 2021-01-13T07:48:54Z

@debsmita1: This pull request references Bugzilla bug 1911269, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1911269: hide "waiting for builds" info message when a build is present

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 kubernetes/test-infra repository.

jerolimov · 2021-01-13T09:59:42Z

frontend/public/components/overview/pods-overview.tsx

+              {t('public~{{waitingPods}} waiting pods with errors', {
+                waitingPods: showWaitingPods ? 'Hide' : 'Show',
+              })}


Hi, this doesn't translate Hide and Show, right? So my suggest is:

Suggested change

{t('public~{{waitingPods}} waiting pods with errors', {

waitingPods: showWaitingPods ? 'Hide' : 'Show',

})}

{showWaitingPods ? t('public~Hide waiting pods with errors') : t('public~Show waiting pods with errors')}

I notice that your translation works fine, but its maybe still easier to translate two separate messages.

jerolimov

Tested this locally. Works fine for the initial build and new builds. i18n looks good.

/lgtm

jerolimov · 2021-01-13T10:56:37Z

/assign @rohitkrai03

divyanshiGupta · 2021-01-13T11:40:53Z

@debsmita1 I see the following issues:

Build Alert is shown first and then the pods every time on going to the resources tab. Ideally we should see the loading component and then the pods like it used to be before.

On scaling down to 0 pods Buid Alert is shown. Ideally only the No pod resources found empty state should be shown like before.

divyanshiGupta · 2021-01-13T11:43:10Z

/lgtm cancel

debsmita1 · 2021-01-15T15:18:37Z

@debsmita1 I see the following issues:

Build Alert is shown first and then the pods every time on going to the resources tab. Ideally we should see the loading component and then the pods like it used to be before.

On scaling down to 0 pods Buid Alert is shown. Ideally only the No pod resources found empty state should be shown like before.

Fixed !

rohitkrai03 · 2021-01-20T13:42:26Z

frontend/public/components/overview/pods-overview.tsx

  const [showWaitingPods, setShowWaitingPods] = React.useState(false);
  const showWaitingForBuildAlert =
-    hasBuildConfig && isDeploymentGeneratedByWebConsole(obj) && pods.some(isPodWithoutImageId);
+    !buildConfigData?.buildConfigs?.[0].builds.some((build) => isComplete(build)) &&


What if there are no buildConfigs in the returns object?

added check for buildConfigs

rohitkrai03 · 2021-01-20T13:43:20Z

frontend/public/components/overview/pods-overview.tsx

+  const PodAlert = () =>
+    showWaitingForBuildAlert ? (
+      <Alert
+        isInline
+        variant="info"
+        title={t('public~Waiting for the build')}
+        actionLinks={
+          <AlertActionLink onClick={() => setShowWaitingPods(!showWaitingPods)}>
+            {showWaitingPods
+              ? t('public~Hide waiting pods with errors')
+              : t('public~Show waiting pods with errors')}
+          </AlertActionLink>
+        }
+      >
+        {t(
+          'public~Waiting for the first build to run successfully. You may temporarily see "ImagePullBackOff" and "ErrImagePull" errors while waiting.',
+        )}
+      </Alert>
+    ) : null;


No need to create a function here, you can just write jsx directly.

Suggested change

const PodAlert = () =>

showWaitingForBuildAlert ? (

<Alert

isInline

variant="info"

title={t('public~Waiting for the build')}

actionLinks={

<AlertActionLink onClick={() => setShowWaitingPods(!showWaitingPods)}>

{showWaitingPods

? t('public~Hide waiting pods with errors')

: t('public~Show waiting pods with errors')}

</AlertActionLink>

}

>

{t(

'public~Waiting for the first build to run successfully. You may temporarily see "ImagePullBackOff" and "ErrImagePull" errors while waiting.',

)}

</Alert>

) : null;

const PodAlert = showWaitingForBuildAlert ? (

<Alert

isInline

variant="info"

title={t('public~Waiting for the build')}

actionLinks={

<AlertActionLink onClick={() => setShowWaitingPods(!showWaitingPods)}>

{showWaitingPods

? t('public~Hide waiting pods with errors')

: t('public~Show waiting pods with errors')}

</AlertActionLink>

}

>

{t(

'public~Waiting for the first build to run successfully. You may temporarily see "ImagePullBackOff" and "ErrImagePull" errors while waiting.',

)}

</Alert>

) : null;

rohitkrai03

/approve

divyanshiGupta

/lgtm

jerolimov · 2021-01-22T09:33:41Z

frontend/public/components/overview/pods-overview.tsx

-  const emptyMessage = emptyText || 'No Pods found for this resource.';
+  const emptyMessage = emptyText || t('public~No Pods found for this resource.');
+
+  const PodAlert = showWaitingForBuildAlert ? (


Sorry, small nit

Suggested change

const PodAlert = showWaitingForBuildAlert ? (

const podAlert = showWaitingForBuildAlert ? (

jerolimov · 2021-01-22T09:34:05Z

frontend/public/components/overview/pods-overview.tsx

-          "ImagePullBackOff" and "ErrImagePull" errors while waiting.
-        </Alert>
-      ) : null}
+      {buildConfigData.loaded && !buildConfigData.loadError && PodAlert}


Suggested change

{buildConfigData.loaded && !buildConfigData.loadError && PodAlert}

{buildConfigData.loaded && !buildConfigData.loadError && podAlert}

jerolimov · 2021-01-22T09:34:23Z

frontend/public/locales/en/public.json

+  "View logs": "View logs",
+  "No Pods found for this resource.": "No Pods found for this resource.",
+  "Waiting for the build": "Waiting for the build",
+  "Hide waiting pods with errors": "Hide waiting pods with errors",
+  "Show waiting pods with errors": "Show waiting pods with errors",
+  "Waiting for the first build to run successfully. You may temporarily see \"ImagePullBackOff\" and \"ErrImagePull\" errors while waiting.": "Waiting for the first build to run successfully. You may temporarily see \"ImagePullBackOff\" and \"ErrImagePull\" errors while waiting.",
+  "View all {{podSize}}": "View all {{podSize}}",


jerolimov · 2021-01-22T09:34:44Z

/retest

jerolimov

Thanks for this quick fix of this nit

/lgtm

openshift-ci-robot · 2021-01-22T10:10:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: debsmita1, divyanshiGupta, 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:

~~frontend/OWNERS~~ [rohitkrai03]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2021-01-22T11:23:24Z

@debsmita1: All pull requests linked via external trackers have merged:

openshift/console#7665

Bugzilla bug 1911269 has been moved to the MODIFIED state.

In response to this:

Bug 1911269: hide "waiting for builds" info message when a build is present

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 kubernetes/test-infra repository.

openshift-ci-robot requested review from jcaianirh and jeff-phillips-18 December 28, 2020 12:42

openshift-ci-robot added the component/core Related to console core functionality label Dec 28, 2020

openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Dec 28, 2020

debsmita1 changed the title ~~hide "waiting for builds" info message when a build is present~~ Bug 1911269: hide "waiting for builds" info message when a build is present Dec 28, 2020

openshift-ci-robot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Dec 28, 2020

openshift-ci-robot requested a review from jerolimov December 28, 2020 12:47

divyanshiGupta reviewed Jan 4, 2021

View reviewed changes

debsmita1 force-pushed the build-message branch from 88812ae to 65280bb Compare January 5, 2021 14:07

rohitkrai03 suggested changes Jan 7, 2021

View reviewed changes

debsmita1 force-pushed the build-message branch from 65280bb to 703e146 Compare January 13, 2021 07:45

debsmita1 requested review from divyanshiGupta and rohitkrai03 January 13, 2021 07:48

jerolimov reviewed Jan 13, 2021

View reviewed changes

debsmita1 force-pushed the build-message branch from 703e146 to d00cccb Compare January 13, 2021 10:44

openshift-ci-robot added the kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated label Jan 13, 2021

jerolimov approved these changes Jan 13, 2021

View reviewed changes

openshift-ci-robot assigned jerolimov Jan 13, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 13, 2021

openshift-ci-robot assigned rohitkrai03 Jan 13, 2021

openshift-ci-robot assigned divyanshiGupta Jan 13, 2021

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 13, 2021

debsmita1 force-pushed the build-message branch from d00cccb to 1c92578 Compare January 15, 2021 14:58

openshift-ci-robot added the component/shared Related to console-shared label Jan 15, 2021

debsmita1 force-pushed the build-message branch from 1c92578 to 6371c3e Compare January 15, 2021 15:36

rohitkrai03 suggested changes Jan 20, 2021

View reviewed changes

debsmita1 force-pushed the build-message branch from 6371c3e to 8955b61 Compare January 22, 2021 07:57

debsmita1 requested a review from rohitkrai03 January 22, 2021 07:58

rohitkrai03 approved these changes Jan 22, 2021

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 2021

divyanshiGupta approved these changes Jan 22, 2021

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2021

jerolimov reviewed Jan 22, 2021

View reviewed changes

hide "waiting for builds" info message when a build is present

8c8e5c2

debsmita1 force-pushed the build-message branch from 8955b61 to 8c8e5c2 Compare January 22, 2021 09:40

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2021

jerolimov approved these changes Jan 22, 2021

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2021

openshift-merge-robot merged commit aed8321 into openshift:master Jan 22, 2021

spadgett added this to the v4.7 milestone Jan 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1911269: hide "waiting for builds" info message when a build is present #7665

Bug 1911269: hide "waiting for builds" info message when a build is present #7665

debsmita1 commented Dec 28, 2020 •

edited

debsmita1 commented Dec 28, 2020

openshift-ci-robot commented Dec 28, 2020

debsmita1 commented Dec 28, 2020

divyanshiGupta Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

jerolimov commented Jan 5, 2021

rohitkrai03 Jan 7, 2021

divyanshiGupta Jan 12, 2021

debsmita1 Jan 13, 2021

openshift-ci-robot commented Jan 13, 2021

jerolimov Jan 13, 2021

jerolimov Jan 13, 2021

debsmita1 Jan 13, 2021

jerolimov left a comment

jerolimov commented Jan 13, 2021

divyanshiGupta commented Jan 13, 2021 •

edited

divyanshiGupta commented Jan 13, 2021

debsmita1 commented Jan 15, 2021

rohitkrai03 Jan 20, 2021

debsmita1 Jan 22, 2021

rohitkrai03 Jan 20, 2021

rohitkrai03 left a comment

divyanshiGupta left a comment

jerolimov Jan 22, 2021

jerolimov Jan 22, 2021

jerolimov Jan 22, 2021

jerolimov commented Jan 22, 2021

jerolimov left a comment

openshift-ci-robot commented Jan 22, 2021

openshift-ci-robot commented Jan 22, 2021

	const PodAlert = showWaitingForBuildAlert ? (
	const podAlert = showWaitingForBuildAlert ? (

	{buildConfigData.loaded && !buildConfigData.loadError && PodAlert}
	{buildConfigData.loaded && !buildConfigData.loadError && podAlert}

Bug 1911269: hide "waiting for builds" info message when a build is present #7665

Bug 1911269: hide "waiting for builds" info message when a build is present #7665

Conversation

debsmita1 commented Dec 28, 2020 • edited

debsmita1 commented Dec 28, 2020

openshift-ci-robot commented Dec 28, 2020

debsmita1 commented Dec 28, 2020

Choose a reason for hiding this comment

openshift-ci-robot commented Jan 4, 2021

jerolimov commented Jan 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jan 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerolimov left a comment

Choose a reason for hiding this comment

jerolimov commented Jan 13, 2021

divyanshiGupta commented Jan 13, 2021 • edited

divyanshiGupta commented Jan 13, 2021

debsmita1 commented Jan 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohitkrai03 left a comment

Choose a reason for hiding this comment

divyanshiGupta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerolimov commented Jan 22, 2021

jerolimov left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jan 22, 2021

openshift-ci-robot commented Jan 22, 2021

debsmita1 commented Dec 28, 2020 •

edited

divyanshiGupta commented Jan 13, 2021 •

edited