Skip to content

Fixed filtering logic for install workflow - #2643

Merged
openshift-merge-robot merged 1 commit into
openshift:masterfrom
cloudbehl:filtering-ocs-nodes
Oct 30, 2019
Merged

Fixed filtering logic for install workflow#2643
openshift-merge-robot merged 1 commit into
openshift:masterfrom
cloudbehl:filtering-ocs-nodes

Conversation

@cloudbehl

@cloudbehl cloudbehl commented Sep 9, 2019

Copy link
Copy Markdown
Contributor

No description provided.

@openshift-ci-robot openshift-ci-robot added component/ceph Related to ceph-storage-plugin size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 9, 2019
@cloudbehl

cloudbehl commented Sep 9, 2019

Copy link
Copy Markdown
Contributor Author

/assign @gnehapk

@cloudbehl

cloudbehl commented Sep 9, 2019

Copy link
Copy Markdown
Contributor Author

/kind bug

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Sep 9, 2019
@cloudbehl

cloudbehl commented Sep 10, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-aws-console-olm
/test e2e-aws-console
/test e2e-aws

@cloudbehl
cloudbehl force-pushed the filtering-ocs-nodes branch 2 times, most recently from b75a7b3 to e2f2884 Compare September 10, 2019 11:20
@afreen23

afreen23 commented Sep 10, 2019

Copy link
Copy Markdown

Its still buggy:
1.The selection node count is not changing
2. The selection of ticks is not happening, one tick gets removed on selection of other

afreen23
afreen23 reviewed Sep 10, 2019
View reviewed changes
Comment thread frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx Outdated

@afreen23 afreen23 Sep 10, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can set directly the nodes count once we get the nodes, why need to use useEffect for that ?

@afreen23 afreen23 Sep 12, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnehapk ^^

@cloudbehl

cloudbehl commented Sep 11, 2019

Copy link
Copy Markdown
Contributor Author

@afreen23 can you do a verification once again. There is still a minor glitch with selecting the filter dropdown. But the values are persistent.

@afreen23

afreen23 commented Sep 12, 2019

Copy link
Copy Markdown

@afreen23 can you do a verification once again. There is still a minor glitch with selecting the filter dropdown. But the values are persistent.

The selection thing needs to be fixed though !
Also, please update the description with the bug that this patch fixes.

afreen23
afreen23 reviewed Sep 12, 2019
View reviewed changes
Comment thread frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx Outdated

@afreen23 afreen23 Sep 12, 2019 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way comparison of objects is not right.

@gnehapk

gnehapk commented Sep 26, 2019

Copy link
Copy Markdown
Contributor

/approve
@afreen23 Can you also test it one?

@gnehapk

gnehapk commented Oct 9, 2019

Copy link
Copy Markdown
Contributor

@cloudbehl Please rebase your PR.

@cloudbehl
cloudbehl force-pushed the filtering-ocs-nodes branch from 7cf2036 to 495c17d Compare October 14, 2019 08:42
@cloudbehl

cloudbehl commented Oct 15, 2019

Copy link
Copy Markdown
Contributor Author

/retest

@cloudbehl
cloudbehl force-pushed the filtering-ocs-nodes branch 3 times, most recently from a5e8da2 to 5b76cc7 Compare October 21, 2019 11:00
@cloudbehl

cloudbehl commented Oct 21, 2019

Copy link
Copy Markdown
Contributor Author

/retest

afreen23
afreen23 reviewed Oct 23, 2019
View reviewed changes
Comment thread frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx Outdated

@afreen23 afreen23 Oct 23, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are not importing getName , its causing test to fail.

@cloudbehl
Fixed filtering logic for install workflow
cb7f8a8 
Signed-off-by: Ankush Behl <cloudbehl@gmail.com>
@cloudbehl
cloudbehl force-pushed the filtering-ocs-nodes branch from 5b76cc7 to cb7f8a8 Compare October 30, 2019 10:53
@afreen23

afreen23 commented Oct 30, 2019

Copy link
Copy Markdown

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2019
@openshift-ci-robot

openshift-ci-robot commented Oct 30, 2019

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afreen23, cloudbehl, gnehapk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-merge-robot
openshift-merge-robot merged commit 180dfd7 into openshift:master Oct 30, 2019
@openshift-ci-robot

openshift-ci-robot commented Oct 30, 2019

Copy link
Copy Markdown
Contributor

@cloudbehl: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws 495c17d002d3eaa9c661c3ff3862a67f7d1e8b16 link /test e2e-aws
ci/prow/e2e-aws-console 495c17d002d3eaa9c661c3ff3862a67f7d1e8b16 link /test e2e-aws-console
ci/prow/e2e-aws-console-olm 5b76cc7b6fbcf368525aee703c00622f0dfac3a3 link /test e2e-aws-console-olm
ci/prow/e2e-gcp 5b76cc7b6fbcf368525aee703c00622f0dfac3a3 link /test e2e-gcp
ci/prow/e2e-gcp-console cb7f8a8 link /test e2e-gcp-console

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

@spadgett spadgett added this to the v4.3 milestone Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gnehapk gnehapk Awaiting requested review from gnehapk
@rawagner rawagner Awaiting requested review from rawagner
1 more reviewer
@afreen23 afreen23 afreen23 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@gnehapk gnehapk

@afreen23 afreen23

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/ceph Related to ceph-storage-plugin kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

v4.3

Development

Successfully merging this pull request may close these issues.

6 participants

@cloudbehl @afreen23 @gnehapk @openshift-ci-robot @spadgett @openshift-merge-robot