Repository: skypilot-org/skypilot
Pull request: [Spot/UX]: Spot dashboard Added delay for ssh_command to initialize
<!-- Describe the changes in this PR -->
Issue: #2123 

User feedback
> Small UX feedback: when I run sky spot dashboard, very often the page that opens first shows some error, and after a few seconds it automatically refreshes and loads. Slightly confusing user experience.

Added a delay between ssh command and webbrowser.open.
<!-- Describe the tests ran -->
<!-- Unit tests (tests/test_*.py) are part of GitHub CI; below are tests that launch on the cloud. -->

Tested (run the relevant ones):

- [x] Code formatting: `bash format.sh`
- [ ] Any manual or new tests for this PR (please specify below)
- [ ] All smoke tests: `pytest tests/test_smoke.py` 
- [ ] Relevant individual smoke tests: `pytest tests/test_smoke.py::test_fill_in_the_name` 
- [ ] Backward compatibility tests: `bash tests/backward_comaptibility_tests.sh`


Problem statement:
[spot dashboard/UI] Page failed to display in the first few seconds after opening
User feedback
> Small UX feedback: when I run sky spot dashboard, very often the page that opens first shows some error, and after a few seconds it automatically refreshes and loads. Slightly confusing user experience.


I noticed that too and not sure what's wrong.

Hints from issue discussion:
This seems to be an issue of ssh_command needing some time to launch before the site is ready for webbrowser.open
A naive solution might be to add a time.sleep() in between.
@MaoZiming makes sense. Do you have time to put in a quick fix? Thanks.

Commit message: spot dashboard: Added delay for ssh_command to initialize

Patch to review:
diff --git a/sky/cli.py b/sky/cli.py
index c69e2c9fbe5..23556c80d62 100644
--- a/sky/cli.py
+++ b/sky/cli.py
@@ -70,6 +70,7 @@
 from sky.utils import ux_utils
 from sky.utils.cli_utils import status_utils
 from sky.usage import usage_lib
+import time
 
 if typing.TYPE_CHECKING:
     from sky.backends import backend as backend_lib
@@ -3736,6 +3737,7 @@ def spot_dashboard(port: Optional[int]):
 
     with subprocess.Popen(ssh_command, shell=True,
                           start_new_session=True) as ssh_process:
+        time.sleep(5)  # Added delay for ssh_command to initialize.
         webbrowser.open(f'http://localhost:{free_port}')
         click.secho(
             f'Dashboard is now available at: http://127.0.0.1:{free_port}',


Code review comments:
diff_hunk: @@ -3736,6 +3737,7 @@ def spot_dashboard(port: Optional[int]):
 
     with subprocess.Popen(ssh_command, shell=True,
                           start_new_session=True) as ssh_process:
+        time.sleep(5)  # Added delay for ssh_command to initialize.
path: sky/cli.py
text: @user1:
Does 2s or 1s work? 5s feels a bit long.

@author:
Reduced to 3s. 
It takes around 2+s on my computer. 
Or maybe we can simply remove webbrowser.open and let user click the link in the prompt.

diff_hunk: @@ -70,6 +70,7 @@
 from sky.utils import ux_utils
 from sky.utils.cli_utils import status_utils
 from sky.usage import usage_lib
+import time
path: sky/cli.py
text: @user1:
nit: we use google styleguide which sorts by section. Standard libs are in the first section, i.e., after L35.

@author:
Moved.