Repository: AgnostiqHQ/covalent
Pull request #1333 (2022-10-12 20:13:11)

Delay returning from covalent start until the endpoints are live
<!--
⚠️ Your pull request title should be short detailed and understandable for all.
⚠️ Increment the version number in the /VERSION file. If this is a bugfix, increment the patch version (the rightmost number), for example 0.18.2 becomes 0.18.3. If it's a new feature, increment the minor version (the middle number), for example 0.18.2 becomes 0.19.0.
⚠️ Add a note to /CHANGELOG.md with your version number and the date, summarizing the changes.
⚠️ If your pull request fixes an open issue, please link to the issue.
⚠️ Rebase the latest changes from the develop branch. Assuming origin points to https://github.com/AgnostiqHQ/covalent, you should run git rebase origin/develop.
-->

- [x] I have added the tests to cover my changes.
- [x] I have updated the documentation and CHANGELOG accordingly.
- [x] I have read the CONTRIBUTING document.

Confirm that the endpoints are live before returning from `covalent start`.

github-actions[bot]: Hello. You may have forgotten to update the changelog!
Please edit [`CHANGELOG.md`](/AgnostiqHQ/covalent/blob/master/CHANGELOG.md) with a one-to-two sentence description of the change in the UNRELEASED section. You may include a small working example for new features.
codecov[bot]: # [Codecov](https://codecov.io/gh/AgnostiqHQ/covalent/pull/1333?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) Report
> Merging [#1333](https://codecov.io/gh/AgnostiqHQ/covalent/pull/1333?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) (194b9b3) into [develop](https://codecov.io/gh/AgnostiqHQ/covalent/commit/2d09daf40c5ad7e16c38c3ba3b4ea43e28710550?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) (2d09daf) will **increase** coverage by `5.69%`.
> The diff coverage is `100.00%`.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://codecov.io/gh/AgnostiqHQ/covalent/pull/1333/graphs/tree.svg?width=650&height=150&src=pr&token=YGHCB3DE4P&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ)](https://codecov.io/gh/AgnostiqHQ/covalent/pull/1333?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ)

```diff
@@             Coverage Diff             @@
##           develop    #1333      +/-   ##
===========================================
+ Coverage    75.62%   81.32%   +5.69%     
===========================================
  Files          140       64      -76     
  Lines         5457     3780    -1677     
  Branches       141        0     -141     
===========================================
- Hits          4127     3074    -1053     
+ Misses        1211      706     -505     
+ Partials       119        0     -119     
```

| Flag | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| Dispatcher | `85.09% <100.00%> (+0.01%)` | :arrow_up: | |
| Functional_Tests | `?` | | |
| SDK | `78.85% <ø> (ø)` | | Carriedforward from [3b88961](https://codecov.io/gh/AgnostiqHQ/covalent/commit/3b88961aa4cd50683bfbec2bb7610828dacbeda6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) |
| UI_Frontend | `?` | | |

*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) to find out more.

</details>

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ac90a4bc..513d09ca 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Exceptions when instantiating executors are handled
+- Covalent start now waits for the server to settle before returning
 
 ## [0.202.0] - 2022-10-11
 
diff --git a/covalent_dispatcher/_cli/service.py b/covalent_dispatcher/_cli/service.py
index 03814d6a..d82ce0b9 100644
--- a/covalent_dispatcher/_cli/service.py
+++ b/covalent_dispatcher/_cli/service.py
@@ -27,12 +27,14 @@ import json
 import os
 import shutil
 import socket
+import time
 from subprocess import DEVNULL, Popen
 from typing import Optional
 
 import click
 import dask.system
 import psutil
+import requests
 from distributed.comm import unparse_address
 from distributed.core import connect, rpc
 
@@ -189,6 +191,16 @@ def _graceful_start(
     with open(pidfile, "w") as PIDFILE:
         PIDFILE.write(str(pid))
 
+    # Wait until the server actually starts listening on the port
+    dispatcher_addr = f"http://localhost:{port}"
+    up = False
+    while not up:
+        try:
+            requests.get(dispatcher_addr, timeout=1)
+            up = True
+        except requests.exceptions.ConnectionError as err:
+            time.sleep(1)
+
     click.echo(f"Covalent server has started at http://localhost:{port}")
     return port
 
@@ -329,16 +341,6 @@ def start(
     set_config("user_interface.port", port)
     set_config("dispatcher.port", port)
 
-    # Wait until the server actually starts listening on the port
-    server_listening = False
-    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-    while not server_listening:
-        try:
-            sock.bind(("localhost", port))
-            sock.close()
-        except OSError:
-            server_listening = True
-
 
 @click.command()
 def stop() -> None:


Test patch:
diff --git a/tests/covalent_dispatcher_tests/_cli/service_test.py b/tests/covalent_dispatcher_tests/_cli/service_test.py
index 04e0f585..18ca8141 100644
--- a/tests/covalent_dispatcher_tests/_cli/service_test.py
+++ b/tests/covalent_dispatcher_tests/_cli/service_test.py
@@ -25,6 +25,7 @@ from unittest.mock import MagicMock, Mock
 
 import mock
 import pytest
+import requests
 from click.testing import CliRunner
 
 from covalent_dispatcher._cli.service import (
@@ -144,6 +145,9 @@ def test_graceful_start_when_pid_absent(mocker):
     )
     popen_mock = mocker.patch("covalent_dispatcher._cli.service.Popen")
     click_echo_mock = mocker.patch("click.echo")
+    requests_mock = mocker.patch(
+        "covalent_dispatcher._cli.service.requests.get",
+    )
 
     with mock.patch("covalent_dispatcher._cli.service.open", mock.mock_open()):
         res = _graceful_start("", "", "output.log", 15, False)
@@ -157,6 +161,35 @@ def test_graceful_start_when_pid_absent(mocker):
     read_pid_mock.assert_called_once()
 
 
+def test_graceful_start_waits_to_return(mocker):
+    """Check that graceful server start function doesn't return until
+    the endpoints are live."""
+
+    read_pid_mock = mocker.patch("covalent_dispatcher._cli.service._read_pid")
+    pid_exists_mock = mocker.patch("psutil.pid_exists", return_value=False)
+    rm_pid_file_mock = mocker.patch("covalent_dispatcher._cli.service._rm_pid_file")
+    next_available_port_mock = mocker.patch(
+        "covalent_dispatcher._cli.service._next_available_port", return_value=1984
+    )
+    popen_mock = mocker.patch("covalent_dispatcher._cli.service.Popen")
+    click_echo_mock = mocker.patch("click.echo")
+    requests_mock = mocker.patch(
+        "covalent_dispatcher._cli.service.requests.get",
+        side_effect=requests.exceptions.ConnectionError(),
+    )
+    sleep_mock = mocker.patch(
+        "covalent_dispatcher._cli.service.time.sleep", side_effect=RuntimeError()
+    )
+
+    with mock.patch("covalent_dispatcher._cli.service.open", mock.mock_open()):
+        with pytest.raises(RuntimeError):
+            res = _graceful_start("", "", "output.log", 15, False)
+
+        pass
+
+    sleep_mock.assert_called_once()
+
+
 def test_graceful_shutdown_running_server(mocker):
     """Test the graceful shutdown functionality."""