Auto-refreshes the run details page #722
Conversation
Auto-refresh is paused when the window loses focus (blur) and is resumed upon re-focus. Autorefresh is permanently terminated if the run has stopped due to failure, error, being skipped, or succeeding.
|
Fixes: #683 |
rileyjbauer
force-pushed
the
autorefresh-run-details
branch
from
c5b0588
to
8c65f33
Compare
|
/test kubeflow-pipeline-build-image |
| expect(setInterval).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| it('does not resume auto refreshing if window loses focus and then regains it but run is finished', async () => { |
There was a problem hiding this comment.
Does it make sense to test the refresh method getting/not getting called rather than just checking setInterval and clearInterval? We're testing the implementation at this point.
There was a problem hiding this comment.
I don't think you're wrong; the interval is just a thing to make sure it refreshes, but refresh is imprecise as it can be called elsewhere, and testing it involves fake timers.
Sort of a side-note, I was previously calling refresh() on focus even if the run had completed to avoid having the slightly more complex code below, although it's technically more correct since it cuts out unnecessary refreshes.
What do you think?
private async _startAutoRefresh(): Promise<void> {
// If the run was not finished last time we checked, check again in case anything changed
// before proceeding to set auto-refresh interval
if (!this.state.runFinished) {
// refresh() updates runFinished's value
await this.refresh();
}
// Only set interval if run has not finished, and verify that the interval is undefined to
// avoid setting multiple intervals
if (!this.state.runFinished && this._interval === undefined) {
this._interval = setInterval(
() => this.refresh(),
this.AUTO_REFRESH_INTERVAL
);
}
}
There was a problem hiding this comment.
I think both ways are fine.
|
/test kubeflow-pipeline-build-image |
| expect(setInterval).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| it('does not resume auto refreshing if window loses focus and then regains it but run is finished', async () => { |
There was a problem hiding this comment.
I think both ways are fine.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yebrahim 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 |
|
@rileyjbauer: The following test failed, say
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. 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. |
* artifacts-gcs on the image build step needs to match the run-notebook step
Auto-refresh is paused when the window loses focus (blur) and is resumed
upon re-focus. Autorefresh is permanently terminated if the run has
stopped due to failure, error, being skipped, or succeeding.
This change is