Repository: AgnostiqHQ/covalent
Pull request #1662 (2023-05-23 17:35:47)

Enhancing error messages
<!--
⚠️ 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.
-->

This PR addresses a comment by @arosen93 about how to better handle the scenario where a server is not started or otherwise unreachable.  Now, the methods `dispatch`, `redispatch`, and `get_result` will print a helpful error message and return `None` if this is encountered.

To test these changes, invoke one of these methods while the server is stopped.

- [ ] 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.


madhur-tandon: @wjcunningham7 you might have to adjust some tests...
I remember adding `test_dispatch_when_no_server_is_running` in https://github.com/AgnostiqHQ/covalent/pull/1601
But there might be others...
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1662?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) Report
> Merging [#1662](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1662?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) (981f2ee) into [develop](https://app.codecov.io/gh/AgnostiqHQ/covalent/commit/e257bd98ea0f40d8a29cfd8f454e933fc8610cea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) (e257bd9) will **increase** coverage by `1.18%`.
> The diff coverage is `47.36%`.

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


[![Impacted file tree graph](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1662/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://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ)

```diff
@@             Coverage Diff             @@
##           develop    #1662      +/-   ##
===========================================
+ Coverage    86.39%   87.57%   +1.18%     
===========================================
  Files          161       85      -76     
  Lines         6841     5134    -1707     
  Branches       151        0     -151     
===========================================
- Hits          5910     4496    -1414     
+ Misses         826      638     -188     
+ Partials       105        0     -105     
```

| Flag | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| Dispatcher | `87.56% <ø> (ø)` | | Carriedforward from [e257bd9](https://app.codecov.io/gh/AgnostiqHQ/covalent/commit/e257bd98ea0f40d8a29cfd8f454e933fc8610cea?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) |
| Functional_Tests | `?` | | |
| SDK | `87.57% <47.36%> (-0.30%)` | :arrow_down: | |
| 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 93f079a3..2ccb00d2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [UNRELEASED]
 
+### Changed
+
+- Error messages are propagated to stdout when the server is not started. These changes are applied to `dispatch`, `redispatch`, and `get_result`.
+
 ### Docs
 
 - Fix typo in GCP Batch executor RTD.
diff --git a/covalent/_dispatcher_plugins/local.py b/covalent/_dispatcher_plugins/local.py
index e14a119a..02d6dc17 100644
--- a/covalent/_dispatcher_plugins/local.py
+++ b/covalent/_dispatcher_plugins/local.py
@@ -149,14 +149,17 @@ class LocalDispatcher(BaseDispatcher):
             lattice_dispatch_id = None
             try:
                 r = requests.post(
-                    submit_dispatch_url, data=json_lattice, params={"disable_run": disable_run}
+                    submit_dispatch_url,
+                    data=json_lattice,
+                    params={"disable_run": disable_run},
+                    timeout=5,
                 )
                 r.raise_for_status()
                 lattice_dispatch_id = r.content.decode("utf-8").strip().replace('"', "")
-            except requests.exceptions.ConnectionError as e:
-                message = f"The Covalent dispatcher server is not running at {dispatcher_addr}. You must start Covalent (with `covalent start`) before dispatching your workflow."
-                app_log.error(message)
-                raise ConnectionError(message) from e
+            except requests.exceptions.ConnectionError:
+                message = f"The Covalent server cannot be reached at {dispatcher_addr}. Local servers can be started using `covalent start` in the terminal. If you are using a remote Covalent server, contact your systems administrator to report an outage."
+                print(message)
+                return
 
             if not disable_run or triggers_data is None:
                 return lattice_dispatch_id
@@ -264,8 +267,16 @@ class LocalDispatcher(BaseDispatcher):
                 dispatch_id, new_args, new_kwargs, replace_electrons, reuse_previous_results
             )
             redispatch_url = f"{dispatcher_addr}/api/redispatch"
-            r = requests.post(redispatch_url, json=body, params={"is_pending": is_pending})
-            r.raise_for_status()
+            try:
+                r = requests.post(
+                    redispatch_url, json=body, params={"is_pending": is_pending}, timeout=5
+                )
+                r.raise_for_status()
+            except requests.exceptions.ConnectionError:
+                message = f"The Covalent server cannot be reached at {dispatcher_addr}. Local servers can be started using `covalent start` in the terminal. If you are using a remote Covalent server, contact your systems administrator to report an outage."
+                print(message)
+                return
+
             return r.content.decode("utf-8").strip().replace('"', "")
 
         return func
diff --git a/covalent/_results_manager/results_manager.py b/covalent/_results_manager/results_manager.py
index 1b0207d0..2a857777 100644
--- a/covalent/_results_manager/results_manager.py
+++ b/covalent/_results_manager/results_manager.py
@@ -74,6 +74,9 @@ def get_result(
 
         raise ex
 
+    except requests.exceptions.ConnectionError:
+        return None
+
     return result
 
 
@@ -111,10 +114,17 @@ def _get_result_from_dispatcher(
     http.mount("http://", adapter)
 
     result_url = f"{dispatcher_addr}/api/result/{dispatch_id}"
-    response = http.get(
-        result_url,
-        params={"wait": bool(int(wait)), "status_only": status_only},
-    )
+
+    try:
+        response = http.get(
+            result_url,
+            params={"wait": bool(int(wait)), "status_only": status_only},
+            timeout=5,
+        )
+    except requests.exceptions.ConnectionError:
+        message = f"The Covalent server cannot be reached at {dispatcher_addr}. Local servers can be started using `covalent start` in the terminal. If you are using a remote Covalent server, contact your systems administrator to report an outage."
+        print(message)
+        raise
 
     if response.status_code == 404:
         raise MissingLatticeRecordError


Test patch:
diff --git a/tests/covalent_tests/dispatcher_plugins/local_test.py b/tests/covalent_tests/dispatcher_plugins/local_test.py
index f0790688..3022ea87 100644
--- a/tests/covalent_tests/dispatcher_plugins/local_test.py
+++ b/tests/covalent_tests/dispatcher_plugins/local_test.py
@@ -76,7 +76,7 @@ def test_get_redispatch_request_body_args_kwargs(mocker):
 
 @pytest.mark.parametrize("is_pending", [True, False])
 @pytest.mark.parametrize(
-    "replace_electrons,expected_arg",
+    "replace_electrons, expected_arg",
     [(None, {}), ({"mock-electron-1": "mock-electron-2"}, {"mock-electron-1": "mock-electron-2"})],
 )
 def test_redispatch(mocker, replace_electrons, expected_arg, is_pending):
@@ -98,6 +98,7 @@ def test_redispatch(mocker, replace_electrons, expected_arg, is_pending):
         "http://mock-config:mock-config/api/redispatch",
         json={"mock-request-body"},
         params={"is_pending": is_pending},
+        timeout=5,
     )
     requests_mock.post().raise_for_status.assert_called_once()
     requests_mock.post().content.decode().strip().replace.assert_called_once_with('"', "")
@@ -105,6 +106,21 @@ def test_redispatch(mocker, replace_electrons, expected_arg, is_pending):
     get_request_body_mock.assert_called_once_with("mock-dispatch-id", (), {}, expected_arg, False)
 
 
+def test_redispatch_unreachable(mocker):
+    """Test the local re-dispatch function when the server is unreachable."""
+
+    mock_dispatch_id = "mock-dispatch-id"
+    dummy_dispatcher_addr = "http://localhost:12345"
+
+    message = f"The Covalent server cannot be reached at {dummy_dispatcher_addr}. Local servers can be started using `covalent start` in the terminal. If you are using a remote Covalent server, contact your systems administrator to report an outage."
+
+    mock_print = mocker.patch("covalent._dispatcher_plugins.local.print")
+
+    LocalDispatcher.redispatch(mock_dispatch_id, dispatcher_addr=dummy_dispatcher_addr)()
+
+    mock_print.assert_called_once_with(message)
+
+
 def test_dispatching_a_non_lattice():
     """test dispatching a non-lattice"""
 
@@ -123,13 +139,15 @@ def test_dispatching_a_non_lattice():
         LocalDispatcher.dispatch(workflow)(1, 2)
 
 
-def test_dispatch_when_no_server_is_running():
+def test_dispatch_when_no_server_is_running(mocker):
     """test dispatching a lattice when no server is running"""
 
     # the test suite is using another port, thus, with the dummy address below
     # the covalent server is not running in some sense.
     dummy_dispatcher_addr = "http://localhost:12345"
 
+    message = f"The Covalent server cannot be reached at {dummy_dispatcher_addr}. Local servers can be started using `covalent start` in the terminal. If you are using a remote Covalent server, contact your systems administrator to report an outage."
+
     @ct.electron
     def task(a, b, c):
         return a + b + c
@@ -138,11 +156,11 @@ def test_dispatch_when_no_server_is_running():
     def workflow(a, b):
         return task(a, b, c=4)
 
-    with pytest.raises(
-        ConnectionError,
-        match=f"The Covalent dispatcher server is not running at {dummy_dispatcher_addr}.",
-    ):
-        LocalDispatcher.dispatch(workflow, dispatcher_addr=dummy_dispatcher_addr)(1, 2)
+    mock_print = mocker.patch("covalent._dispatcher_plugins.local.print")
+
+    LocalDispatcher.dispatch(workflow, dispatcher_addr=dummy_dispatcher_addr)(1, 2)
+
+    mock_print.assert_called_once_with(message)
 
 
 def test_dispatcher_submit_api(mocker):
diff --git a/tests/covalent_tests/results_manager_tests/results_manager_test.py b/tests/covalent_tests/results_manager_tests/results_manager_test.py
index f6a97172..7134cb55 100644
--- a/tests/covalent_tests/results_manager_tests/results_manager_test.py
+++ b/tests/covalent_tests/results_manager_tests/results_manager_test.py
@@ -24,12 +24,30 @@ from http.client import HTTPMessage
 from unittest.mock import ANY, MagicMock, Mock, call
 
 import pytest
+import requests
 
 from covalent._results_manager import wait
-from covalent._results_manager.results_manager import _get_result_from_dispatcher, cancel
+from covalent._results_manager.results_manager import (
+    _get_result_from_dispatcher,
+    cancel,
+    get_result,
+)
 from covalent._shared_files.config import get_config
 
-DISPATCH_ID = "91c3ee18-5f2d-44ee-ac2a-39b79cf56646"
+
+def test_get_result_unreachable_dispatcher(mocker):
+    """
+    Test that get_result returns None when
+    the dispatcher server is unreachable.
+    """
+    mock_dispatch_id = "mock_dispatch_id"
+
+    mocker.patch(
+        "covalent._results_manager.results_manager._get_result_from_dispatcher",
+        side_effect=requests.exceptions.ConnectionError,
+    )
+
+    assert get_result(mock_dispatch_id) is None
 
 
 @pytest.mark.parametrize(
@@ -67,6 +85,35 @@ def test_get_result_from_dispatcher(mocker, dispatcher_addr):
     )
 
 
+def test_get_result_from_dispatcher_unreachable(mocker):
+    """
+    Test that _get_result_from_dispatcher raises an exception when
+    the dispatcher server is unreachable.
+    """
+
+    # TODO: Will need to edit this once `_get_result_from_dispatcher` is fixed
+    # to actually throw an exception when the dispatcher server is unreachable
+    # instead of just hanging.
+
+    mock_dispatcher_addr = "mock_dispatcher_addr"
+    mock_dispatch_id = "mock_dispatch_id"
+
+    message = f"The Covalent server cannot be reached at {mock_dispatcher_addr}. Local servers can be started using `covalent start` in the terminal. If you are using a remote Covalent server, contact your systems administrator to report an outage."
+
+    mocker.patch("covalent._results_manager.results_manager.HTTPAdapter")
+    mock_session = mocker.patch("covalent._results_manager.results_manager.requests.Session")
+    mock_session.return_value.get.side_effect = requests.exceptions.ConnectionError
+
+    mock_print = mocker.patch("covalent._results_manager.results_manager.print")
+
+    with pytest.raises(requests.exceptions.ConnectionError):
+        _get_result_from_dispatcher(
+            mock_dispatch_id, wait=wait.LONG, dispatcher_addr=mock_dispatcher_addr
+        )
+
+    mock_print.assert_called_once_with(message)
+
+
 def test_cancel_with_single_task_id(mocker):
     mock_get_config = mocker.patch("covalent._results_manager.results_manager.get_config")
     mock_request_post = mocker.patch(