Check Tensorboard Instance #3189
Conversation
|
@jingzhang36 when you use the plugin, it has a button to publish your demo to Youtube as unlisted video. So far I've found that to be the easiest way to share. |
| this.state.tensorboardReady | ||
| ? '' | ||
| : 'Tensorboard is starting, and you may need to wait for a few minutes.' |
There was a problem hiding this comment.
What does this look like? Can you share a demo?
There was a problem hiding this comment.
A tooltip like thing. https://drive.google.com/file/d/1t4A1jjUTa63whv97gZ2IXgp21GT2U_Dq/view
There was a problem hiding this comment.
Thanks for the demo!
I worry the tooltip shows too slowly, it might be hard to notice if someone just quickly clicks the button.
I think some simple text warning at the bottom of under the button works great.
(because the warning may show up and disappear, if it is shown on the top of the button, that will change the button's position on the screen. So prefer on the bottom.)
Done. https://drive.google.com/file/d/1-PNljQz0OxwJp0doeRVZBYznhVBVFj9n/view --Jing
|
For mocking fetch, I think I prefer you move the fetch to Apis class, together with other tensorboard related apis and mock the corresponding Apis' method. |
|
/test kubeflow-pipeline-sample-test |
| this.state.tensorboardReady | ||
| ? '' | ||
| : 'Tensorboard is starting, and you may need to wait for a few minutes.' |
There was a problem hiding this comment.
Thanks for the demo!
I worry the tooltip shows too slowly, it might be hard to notice if someone just quickly clicks the button.
I think some simple text warning at the bottom of under the button works great.
(because the warning may show up and disappear, if it is shown on the top of the button, that will change the button's position on the screen. So prefer on the bottom.)
Done. https://drive.google.com/file/d/1-PNljQz0OxwJp0doeRVZBYznhVBVFj9n/view --Jing
| await TestUtils.flushPromises(); | ||
| jest.runOnlyPendingTimers(); | ||
| await TestUtils.flushPromises(); |
There was a problem hiding this comment.
Can we wrap these into a helper?
like flushPromisesAndTimers?
|
Fixes #2482 |
|
/assign @Bobgy |
| // If pod address is not null and tensorboard pod doesn't seem to be read, pull status again | ||
| if (this.state.podAddress && !this.state.tensorboardReady) { | ||
| Apis.isTensorboardPodReady( | ||
| 'apis/v1beta1/_proxy/' + this.state.podAddress.replace(/(^\w+:|^)\/\//, ''), |
There was a problem hiding this comment.
nit: Can we have a comment explaining the regex here?
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Bobgy 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 |
* Add tensorboard pod status check * print debug log * Need to keep pulling status if the first attemp to ping tb instance returns non-200 * add test * revise comment * use fetch instead of axios to send http head request * remove accidentally checked-in files * format * address comments * format * address comments * comments
If instance is not fully up, we (1) disable open tensorboard button and (2) instead show a message asking user to wait. When the instance is up and ready, we show open tensorboard button.
Demo link (sorry this link might not be accessible to public) https://drive.google.com/file/d/1t4A1jjUTa63whv97gZ2IXgp21GT2U_Dq/view
This change is