-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add refresh button to widgets UI #741
Add refresh button to widgets UI #741
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #741 +/- ##
=======================================
Coverage ? 94.12%
=======================================
Files ? 36
Lines ? 2417
Branches ? 0
=======================================
Hits ? 2275
Misses ? 142
Partials ? 0 ☔ View full report in Codecov by Sentry. |
@ChristianZaccaria I thought that we were also going to add polling as well as the refresh button? |
@Bobbins228 I left a comment in the Jira summarizing the team discussion. TLDR: Option 0 and 1 are preferred.
Although thinking about it, we could definitely alter I.e.,:
|
@ChristianZaccaria nevermind we should stick to our initial vote of option 0 |
e95e6d5
to
87735b4
Compare
0c4bebc
to
fa754c0
Compare
/lgtm tested this out and works as intended |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm Tested and worked as expected
@ChristianZaccaria can you rebase this when you get the chance and we can approve again? |
fa754c0
to
41f733a
Compare
Rebased |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Bobbins228, KPostOffice 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 |
a5a229c
into
project-codeflare:main
Issue link
Jira: https://issues.redhat.com/browse/RHOAIENG-14057
What changes have been made
Added a refresh button to the UI:
When the button is clicked, the DataFrame is refreshed. Refreshing the list of RayClusters, the spec, and the status of the RayClusters in the DataFrame.
Additionally, when the button is clicked, the button is disabled until the K8s API request for the list of RayClusters has returned. - This usually takes around 1 second.
Removed redundant code. - Using
self.namespace
variable as opposed to retrieving the namespace from the dataframe.Updated Sphinx docs.
Added UI e2e test case.
Small refactor to separate concerns.
Verification steps
poetry build
to generate a new wheel.pip install
.view_clusters()
We can also test to refresh the DataFrame while the RayCluster has the status
Failed
. Once ready we can refresh the DataFrame to show current status, eventually beingReady
.Checks