Bug 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable #6572

divyanshiGupta · 2020-09-09T13:08:35Z

Jira: https://issues.redhat.com/browse/ODC-4360
Fixes #5190

Screenshot:

Unit Tests Coverage:

/kind bug

divyanshiGupta · 2020-09-09T13:08:49Z

/assign @rohitkrai03

abhinandan13jan · 2020-09-09T13:17:06Z

frontend/packages/dev-console/src/components/helm/HelmInstallUpgradePage.tsx

-      } catch {} // eslint-disable-line no-empty
+      } catch (e) {
+        error = e;
+      } // eslint-disable-line no-empty
      if (ignore) return;


just a query. Why do we have this ignore check after the coFetch and not before it?

Okay I will try to explain it briefly. So we do this to prevent updating a state of an unmounted component and as you can see the state is being updated only after the API call. If a component is unmounted then anyways a new API call cannot be made but this helps when a call is already made but the response has not yet been received and the component unmounts before that, so when the call returns we prevent the state from being updated.

openshift-ci-robot · 2020-09-09T13:18:37Z

@divyanshiGupta: This pull request references Bugzilla bug 1877368, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

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

In response to this:

Bug 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable

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.

rohitkrai03

I think chart version dropdown should be hidden if there's no chart version present. Empty dropdown feels odd here. @parvathyvr WDYT?

andrewballantyne · 2020-09-10T15:12:46Z

I think chart version dropdown should be hidden if there's no chart version present. Empty dropdown feels odd here. @parvathyvr WDYT?

Do we have placeholders? I wonder if "No versions available" or something akin to that is better... Just wondering if removing the input item, labels et all is going to leave more of a void in the UI.

parvathyvr

I agree with @rohitkrai03 that the empty dropdown looks odd.In the original mockup screen attached in the bug ODC-4360 I had the Chart Version dropdown although disabled ,showing the latest chart version/app version.But if we do not have this data to show, my opinion would be in lines with what @andrewballantyne suggested above.I think if we do not have data to show in the ‘Chart Version’ dropdown we can show ‘Not available’ in the disabled dropdown. CC @beaumorley

divyanshiGupta · 2020-09-10T15:42:17Z

@parvathyvr I will add the No versions available placeholder in case of versions not being available keeping other things same as shown in the above screenshot.

rohitkrai03 · 2020-09-10T15:51:39Z

@parvathyvr Do you think we should also add the http error that we get back from backend to the alert. I tried it with a private helm chart that is not accessible and it looks something like this -

openshift-ci-robot · 2020-09-10T16:25:40Z

@divyanshiGupta: This pull request references Bugzilla bug 1877368, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.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 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable

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.

parvathyvr · 2020-09-10T17:13:57Z

@rohitkrai03 I think giving more info about why the Helm chart is unavailable is a good addition!

@parvathyvr Do you think we should also add the http error that we get back from backend to the alert. I tried it with a private helm chart that is not accessible and it looks something like this -

openshift-ci-robot · 2020-09-10T17:37:03Z

@divyanshiGupta: This pull request references Bugzilla bug 1877368, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.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 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable

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.

divyanshiGupta · 2020-09-10T17:37:18Z

@rohitkrai03 I think giving more info about why the Helm chart is unavailable is a good addition!

@parvathyvr Do you think we should also add the http error that we get back from backend to the alert. I tried it with a private helm chart that is not accessible and it looks something like this -

@rohitkrai03 @parvathyvr done. PTAL.

openshift-ci-robot · 2020-09-10T17:39:23Z

@divyanshiGupta: This pull request references Bugzilla bug 1877368, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.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 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable

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.

rohitkrai03

/approve

parvathyvr

LGTM!

rohitkrai03 · 2020-09-16T15:22:01Z

/lgtm

openshift-ci-robot · 2020-09-16T15:22:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divyanshiGupta, parvathyvr, 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/packages/dev-console/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-bot · 2020-09-16T18:13:19Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-16T23:37:59Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-16T23:51:10Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T02:01:03Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T04:24:12Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T04:37:03Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T05:03:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T05:43:14Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T06:21:02Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T06:47:03Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T08:05:01Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-17T10:15:02Z

/retest

Please review the full test history for this PR and help us cut down flakes.

rohitkrai03 · 2020-09-17T15:01:29Z

/retest

openshift-ci-robot · 2020-09-17T19:38:50Z

@divyanshiGupta: This pull request references Bugzilla bug 1877368, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.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 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable

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-bot · 2020-09-17T20:51:30Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-18T00:31:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-18T00:58:00Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-18T03:21:00Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-18T03:35:24Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-09-18T07:28:40Z

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

openshift/console#6572

Bugzilla bug 1877368 has been moved to the MODIFIED state.

In response to this:

Bug 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable

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 added the kind/bug Categorizes issue or PR as related to a bug. label Sep 9, 2020

openshift-ci-robot assigned rohitkrai03 Sep 9, 2020

openshift-ci-robot added the component/dev-console Related to dev-console label Sep 9, 2020

openshift-ci-robot requested review from andrewballantyne and nemesis09 September 9, 2020 13:08

abhinandan13jan reviewed Sep 9, 2020

View reviewed changes

divyanshiGupta changed the title ~~Show error alert in Helm Chart Install form if chart is not reachable~~ Bug 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable Sep 9, 2020

openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium 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 Sep 9, 2020

divyanshiGupta force-pushed the jira-issue#4360 branch from 9beefa1 to 0d9511c Compare September 9, 2020 13:34

rohitkrai03 suggested changes Sep 10, 2020

View reviewed changes

parvathyvr reviewed Sep 10, 2020

View reviewed changes

divyanshiGupta force-pushed the jira-issue#4360 branch from 0d9511c to d83f432 Compare September 10, 2020 16:23

divyanshiGupta force-pushed the jira-issue#4360 branch from d83f432 to 57dc4a7 Compare September 10, 2020 17:36

divyanshiGupta force-pushed the jira-issue#4360 branch from 57dc4a7 to 2b40b80 Compare September 10, 2020 17:39

Show error alert if chart is not reachable

2b40b80

rohitkrai03 approved these changes Sep 11, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 11, 2020

parvathyvr approved these changes Sep 11, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 16, 2020

rohitkrai03 mentioned this pull request Sep 17, 2020

HELM - Console displays 'Install Helm Chart' screen w/o values.yaml when index.yaml points to invalid tgz path #5190

Closed

openshift-merge-robot merged commit 9e71e2a into openshift:master Sep 18, 2020

spadgett added this to the v4.6 milestone Sep 30, 2020

Bug 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable #6572

Bug 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable #6572

Conversation

divyanshiGupta commented Sep 9, 2020 • edited by spadgett

divyanshiGupta commented Sep 9, 2020

abhinandan13jan Sep 9, 2020

Choose a reason for hiding this comment

divyanshiGupta Sep 9, 2020 • edited

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 9, 2020

rohitkrai03 left a comment

Choose a reason for hiding this comment

andrewballantyne commented Sep 10, 2020

parvathyvr left a comment

Choose a reason for hiding this comment

divyanshiGupta commented Sep 10, 2020

rohitkrai03 commented Sep 10, 2020 • edited

openshift-ci-robot commented Sep 10, 2020

parvathyvr commented Sep 10, 2020

openshift-ci-robot commented Sep 10, 2020

divyanshiGupta commented Sep 10, 2020

openshift-ci-robot commented Sep 10, 2020

rohitkrai03 left a comment

Choose a reason for hiding this comment

parvathyvr left a comment

Choose a reason for hiding this comment

rohitkrai03 commented Sep 16, 2020

openshift-ci-robot commented Sep 16, 2020

openshift-bot commented Sep 16, 2020

openshift-bot commented Sep 16, 2020

openshift-bot commented Sep 16, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

rohitkrai03 commented Sep 17, 2020

openshift-ci-robot commented Sep 17, 2020

openshift-bot commented Sep 17, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

divyanshiGupta commented Sep 9, 2020 •

edited by spadgett

divyanshiGupta Sep 9, 2020 •

edited

rohitkrai03 commented Sep 10, 2020 •

edited