Repository: AgnostiqHQ/covalent (python)

Issue:
Determine if covalent subprocess starts successfully
## Description

The covalent CLI currently starts a FastAPI server as a subprocess when `covalent start` is invoked, the PID is persisted to disk subsequently and `covalent status` just checks the existence of this PID file.

> The relevant methods can be found in: `covalent_dispacther._cli.service.py`

Any errors that may be present when starting the subprocess are currently suppressed and the PID file is persisted even if the subprocess fails to start. As a result we want to then:

- Not persist the PID file if the subprocess failed to start (so `covalent status` does not get confused)
- Display the subprocess error to the end user (more on UX below)
- Enhance `covalent status` to determine if the covalent process is stopped, swapped out of memory, or in a zombie state

Pull request:
[UnitaryHack] Handle process start exception, process status zombie, process stopped
<!--
⚠️ 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.
-->
closes #1648 
- [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.

Acceptance Criteria
- [x]  Display an error message and don't persist PID if there is an error when starting subprocess
- [x]  Display additional information in covalent status if subprocess is unhealthy
- [x]  Must satisfy UX provided below
- [x]  Update tests and ensure they pass locally and in the PR

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 40828c44..1b1a55f4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [UNRELEASED]
 
+- Add CLI status for zombie, stopped process.
+
 ## [0.227.0-rc.0] - 2023-06-13
 
 ### Authors
@@ -93,6 +95,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - 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/_cli/service.py b/covalent_dispatcher/_cli/service.py
index b6ffb6b2..ebf345c5 100644
--- a/covalent_dispatcher/_cli/service.py
+++ b/covalent_dispatcher/_cli/service.py
@@ -29,6 +29,7 @@ import shutil
 import socket
 import sys
 import time
+import traceback
 from pathlib import Path
 from subprocess import DEVNULL, Popen
 from typing import Optional
@@ -56,6 +57,8 @@ MIGRATION_WARNING_MSG = "Covalent not started. The database needs to be upgraded
 MIGRATION_COMMAND_MSG = (
     '   (use "covalent db migrate" to run database migrations and then retry "covalent start")'
 )
+ZOMBIE_PROCESS_STATUS_MSG = "Covalent server is unhealthy: Process is in zombie status"
+STOPPED_PROCESS_STATUS_MSG = "Covalent server is unhealthy: Process is in stopped status"
 
 
 def _read_pid(filename: str) -> int:
@@ -400,9 +403,24 @@ def start(
     else:
         set_config("sdk.no_cluster", "true")
 
-    port = _graceful_start(
-        UI_SRVDIR, UI_PIDFILE, UI_LOGFILE, port, no_cluster, develop, no_triggers, triggers_only
-    )
+    try:
+        port = _graceful_start(
+            UI_SRVDIR,
+            UI_PIDFILE,
+            UI_LOGFILE,
+            port,
+            no_cluster,
+            develop,
+            no_triggers,
+            triggers_only,
+        )
+    except Exception:
+        click.secho("Error: ", fg="red")
+        click.secho(
+            "Covalent was unable to start due to the following error: ", fg="red", bold=True
+        )
+        click.secho(traceback.format_exc(), fg="lightgrey")
+        return ctx.exit(1)
     set_config("user_interface.port", port)
     set_config("dispatcher.port", port)
 
@@ -451,7 +469,11 @@ def status() -> None:
     """
 
     pid = _read_pid(UI_PIDFILE)
-    if _read_pid(UI_PIDFILE) != -1 and psutil.pid_exists(pid):
+    if psutil.pid_exists(pid) and psutil.Process(pid).status() == psutil.STATUS_ZOMBIE:
+        click.echo(ZOMBIE_PROCESS_STATUS_MSG)
+    elif psutil.pid_exists(pid) and psutil.Process(pid).status() == psutil.STATUS_STOPPED:
+        click.echo(STOPPED_PROCESS_STATUS_MSG)
+    elif _read_pid(UI_PIDFILE) != -1 and psutil.pid_exists(pid):
         ui_port = get_config("user_interface.port")
         click.echo(f"Covalent server is running at http://localhost:{ui_port}.")
     else:


Test patch:
diff --git a/tests/covalent_dispatcher_tests/_cli/service_test.py b/tests/covalent_dispatcher_tests/_cli/service_test.py
index fb06cb3f..f0e452c8 100644
--- a/tests/covalent_dispatcher_tests/_cli/service_test.py
+++ b/tests/covalent_dispatcher_tests/_cli/service_test.py
@@ -28,6 +28,7 @@ import venv
 from unittest import mock
 from unittest.mock import MagicMock, Mock
 
+import psutil
 import pytest
 import requests
 from click.testing import CliRunner
@@ -35,6 +36,8 @@ from click.testing import CliRunner
 from covalent_dispatcher._cli.service import (
     MIGRATION_COMMAND_MSG,
     MIGRATION_WARNING_MSG,
+    STOPPED_PROCESS_STATUS_MSG,
+    ZOMBIE_PROCESS_STATUS_MSG,
     _graceful_shutdown,
     _graceful_start,
     _is_server_running,
@@ -55,6 +58,8 @@ from covalent_dispatcher._db.datastore import DataStore
 
 STOPPED_SERVER_STATUS_ECHO = "Covalent server is stopped.\n"
 RUNNING_SERVER_STATUS_ECHO = "Covalent server is running at http://localhost:42.\n"
+STOPPED_PROCESS_STATUS_ECHO = STOPPED_PROCESS_STATUS_MSG + "\n"
+ZOMBIE_PROCESS_STATUS_ECHO = ZOMBIE_PROCESS_STATUS_MSG + "\n"
 
 
 def has_conda():
@@ -447,26 +452,31 @@ def test_restart_preserves_nocluster(mocker, port_tag, port, pid, server, no_clu
 
 
 @pytest.mark.parametrize(
-    "port_val,pid,echo_output,file_removed,pid_exists",
+    "port_val,pid,echo_output,file_removed,pid_exists,process_status",
     [
-        (None, -1, STOPPED_SERVER_STATUS_ECHO, True, False),
-        (42, 42, RUNNING_SERVER_STATUS_ECHO, False, True),
-        (42, 42, STOPPED_SERVER_STATUS_ECHO, True, False),
+        (None, -1, STOPPED_SERVER_STATUS_ECHO, True, False, None),
+        (42, 42, RUNNING_SERVER_STATUS_ECHO, False, True, psutil.STATUS_RUNNING),
+        (42, 42, STOPPED_SERVER_STATUS_ECHO, True, False, None),
+        (42, 42, ZOMBIE_PROCESS_STATUS_ECHO, False, True, psutil.STATUS_ZOMBIE),
+        (42, 42, STOPPED_PROCESS_STATUS_ECHO, False, True, psutil.STATUS_STOPPED),
     ],
 )
-def test_status(mocker, port_val, pid, echo_output, file_removed, pid_exists):
+def test_status(mocker, port_val, pid, echo_output, file_removed, pid_exists, process_status):
     """Test covalent status command."""
 
     mocker.patch("covalent_dispatcher._cli.service.get_config", return_value=port_val)
     mocker.patch("covalent_dispatcher._cli.service._read_pid", return_value=pid)
     mocker.patch("psutil.pid_exists", return_value=pid_exists)
+    mocker.patch("psutil.Process.status", return_value=process_status)
     rm_pid_file_mock = mocker.patch("covalent_dispatcher._cli.service._rm_pid_file")
+    process_mock = Mock(spec=psutil.Process)
+    process_mock.status.return_value = process_status
+    with mocker.patch("psutil.Process", return_value=process_mock):
+        runner = CliRunner()
+        res = runner.invoke(status)
 
-    runner = CliRunner()
-    res = runner.invoke(status)
-
-    assert res.output == echo_output
-    assert rm_pid_file_mock.called is file_removed
+        assert res.output == echo_output
+        assert rm_pid_file_mock.called is file_removed
 
 
 def test_is_server_running(mocker):
@@ -1134,3 +1144,18 @@ def test_terminate_child_processes(mocker):
     _terminate_child_processes(1)
 
     psutil_process_mock.assert_called_with(1)
+
+
+def test_graceful_start_permission_exception(mocker):
+    graceful_start_mock = mocker.patch(
+        "covalent_dispatcher._cli.service._graceful_start",
+        side_effect=PermissionError("Permission denied"),
+    )
+    click_secho_mock = mocker.patch("covalent_dispatcher._cli.service.click.secho")
+
+    runner = CliRunner()
+    result = runner.invoke(start)
+    assert result.exit_code == 1
+
+    assert graceful_start_mock.called_once()
+    assert click_secho_mock.call_count == 3