Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create watch k8s hook #3443

Merged
merged 1 commit into from Mar 19, 2020

Conversation

rawagner
Copy link
Contributor

@rawagner rawagner commented Nov 18, 2019

This hook is intended to be used in functional components as alternative to wrapping them in Firehose.

I've created two versions

  • first one for fetching one resource - if we have more resources to fetch, we will call the hook multiple times. This aligns better with how hooks are generally used
  • the other one for fetching multiple resources at once since there are cases when the resources are defined dynamically (ie from plugins) - this hook is pretty similar to how Firehose works

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 18, 2019
@rawagner
Copy link
Contributor Author

/hold

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 18, 2019
@openshift-ci-robot openshift-ci-robot added component/core Related to console core functionality component/dashboard Related to dashboard component/shared Related to console-shared approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 18, 2019
@rawagner rawagner force-pushed the watch_k8s_hookj branch 4 times, most recently from 7abc858 to 71d3e09 Compare November 25, 2019 08:55
@rawagner rawagner changed the title WIP: Create watch k8s hook Create watch k8s hook Dec 2, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 2, 2019
@rawagner
Copy link
Contributor Author

rawagner commented Dec 2, 2019

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2019
@rawagner rawagner force-pushed the watch_k8s_hookj branch 2 times, most recently from 9dca8c9 to a494421 Compare December 3, 2019 12:09
@rawagner
Copy link
Contributor Author

rawagner commented Dec 3, 2019

@spadgett I'd like to get your feedback on this one

@rawagner
Copy link
Contributor Author

/assign @spadgett @benjaminapetersen

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 13, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2019
@rawagner
Copy link
Contributor Author

rebased and renamed useWatchK8s to useK8sWatch to align with useK8sGet hook naming.

note that I converted just a few cards to use this hook as example, we can use the hook for all cards once approved

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2020
@vojtechszocs
Copy link
Contributor

The recent change should fix remaining type related issues.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2020
@vojtechszocs
Copy link
Contributor

1) Cluster Dashboard Inventory Card : has all items
   NoSuchElementError: No element found using locator: By(css selector, [data-test-id="inventory-card"])

Need to retain data-test-id attribute in InventoryCard render function:

-<DashboardCard>
+<DashboardCard data-test-id="inventory-card">

@openshift-bot
Copy link
Contributor

/retest

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

13 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2020
@rawagner
Copy link
Contributor Author

/retest

@vojtechszocs
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 19, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rawagner, vojtechszocs

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 97fa3b4 into openshift:master Mar 19, 2020
@spadgett spadgett added this to the v4.5 milestone Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dashboard Related to dashboard component/kubevirt Related to kubevirt-plugin component/metal3 Related to metal3-plugin component/sdk Related to console-plugin-sdk component/shared Related to console-shared lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants