From 5a82809630a665219cba15a85d57d681b05e7087 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Mon, 12 May 2025 16:58:16 -0700 Subject: [PATCH 01/23] add validation for non remote dependencies --- devservices/configs/service_config.py | 12 ++++ devservices/utils/services.py | 8 +-- tests/configs/test_service_config.py | 100 +++++++++++++++++++++++--- 3 files changed, 103 insertions(+), 17 deletions(-) diff --git a/devservices/configs/service_config.py b/devservices/configs/service_config.py index 6b1fcf34..62e459b6 100644 --- a/devservices/configs/service_config.py +++ b/devservices/configs/service_config.py @@ -84,6 +84,8 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: ) service_config_data = config.get("x-sentry-service-config") + docker_compose_services = config.get("services", {}).keys() + valid_dependency_keys = {field.name for field in fields(Dependency)} dependencies = {} @@ -106,6 +108,16 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: f"Error parsing service dependencies: {type_error}" ) from type_error + # Validate that all non-remote dependencies are defined in docker-compose services + for dependency_name, dependency in dependencies.items(): + if ( + dependency.remote is None + and dependency_name not in docker_compose_services + ): + raise ConfigValidationError( + f"Dependency '{dependency_name}' is not remote but is not defined in docker-compose services" + ) + service_config = ServiceConfig( version=service_config_data.get("version"), service_name=service_config_data.get("service_name"), diff --git a/devservices/utils/services.py b/devservices/utils/services.py index f8bb72a3..005bbd2b 100644 --- a/devservices/utils/services.py +++ b/devservices/utils/services.py @@ -8,7 +8,6 @@ from devservices.exceptions import ConfigParseError from devservices.exceptions import ConfigValidationError from devservices.exceptions import ServiceNotFoundError -from devservices.utils.console import Console from devservices.utils.devenv import get_coderoot @@ -23,16 +22,13 @@ def get_local_services(coderoot: str) -> list[Service]: """Get a list of services in the coderoot.""" from devservices.configs.service_config import load_service_config_from_file - console = Console() - services = [] for repo in os.listdir(coderoot): repo_path = os.path.join(coderoot, repo) try: service_config = load_service_config_from_file(repo_path) - except (ConfigParseError, ConfigValidationError) as e: - console.warning(f"{repo} was found with an invalid config: {e}") - continue + except (ConfigParseError, ConfigValidationError): + raise except ConfigNotFoundError: # Ignore repos that don't have devservices configs continue diff --git a/tests/configs/test_service_config.py b/tests/configs/test_service_config.py index bcc02933..e6797630 100644 --- a/tests/configs/test_service_config.py +++ b/tests/configs/test_service_config.py @@ -70,7 +70,13 @@ def test_load_service_config_from_file( "service_name": service_name, "dependencies": {key: value for key, value in dependencies.items()}, "modes": {key: value for key, value in modes.items()}, - } + }, + "services": { + key: { + "image": key, + } + for key in dependencies.keys() + }, } create_config_file(tmp_path, config) @@ -95,7 +101,8 @@ def test_load_service_config_from_file_no_dependencies(tmp_path: Path) -> None: "version": 0.1, "service_name": "example-service", "modes": {"default": []}, - } + }, + "services": {}, } create_config_file(tmp_path, config) @@ -126,7 +133,12 @@ def test_load_service_config_from_file_invalid_version(tmp_path: Path) -> None: "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -142,7 +154,12 @@ def test_load_service_config_from_file_missing_version(tmp_path: Path) -> None: "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -159,7 +176,12 @@ def test_load_service_config_from_file_missing_service_name(tmp_path: Path) -> N "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -177,7 +199,12 @@ def test_load_service_config_from_file_invalid_dependency(tmp_path: Path) -> Non "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency", "unknown-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -198,7 +225,12 @@ def test_load_service_config_from_file_missing_default_mode(tmp_path: Path) -> N "example-dependency": {"description": "Example dependency"} }, "modes": {"custom": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -215,7 +247,12 @@ def test_load_service_config_from_file_no_modes(tmp_path: Path) -> None: "dependencies": { "example-dependency": {"description": "Example dependency"} }, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -224,6 +261,32 @@ def test_load_service_config_from_file_no_modes(tmp_path: Path) -> None: assert str(e.value) == "Default mode is required in service config" +def test_load_service_config_from_file_remote_dependency_not_in_services( + tmp_path: Path, +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "example-dependency": { + "description": "Example dependency", + "remote": { + "repo_name": "example-dependency", + "repo_link": "https://github.com/example/example-dependency", + "branch": "main", + }, + }, + }, + "modes": {"default": ["example-dependency"]}, + }, + "services": {}, + } + create_config_file(tmp_path, config) + + load_service_config_from_file(str(tmp_path)) + + def test_load_service_config_from_file_invalid_dependencies(tmp_path: Path) -> None: config = { "x-sentry-service-config": { @@ -236,7 +299,12 @@ def test_load_service_config_from_file_invalid_dependencies(tmp_path: Path) -> N } }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -260,7 +328,12 @@ def test_load_service_config_from_file_invalid_modes(tmp_path: Path) -> None: "default": ["example-dependency"], "custom": "example-dependency", }, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -280,7 +353,12 @@ def test_load_service_config_from_file_no_x_sentry_service_config( "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) From 6ecedd55a3b9c410a5ca527ac84b9fdfe3bcf7a9 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Mon, 12 May 2025 17:09:26 -0700 Subject: [PATCH 02/23] fix some tests --- devservices/utils/services.py | 4 ++++ tests/utils/test_services.py | 11 +++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/devservices/utils/services.py b/devservices/utils/services.py index 005bbd2b..6c96d9a7 100644 --- a/devservices/utils/services.py +++ b/devservices/utils/services.py @@ -8,6 +8,7 @@ from devservices.exceptions import ConfigParseError from devservices.exceptions import ConfigValidationError from devservices.exceptions import ServiceNotFoundError +from devservices.utils.console import Console from devservices.utils.devenv import get_coderoot @@ -22,12 +23,15 @@ def get_local_services(coderoot: str) -> list[Service]: """Get a list of services in the coderoot.""" from devservices.configs.service_config import load_service_config_from_file + console = Console() + services = [] for repo in os.listdir(coderoot): repo_path = os.path.join(coderoot, repo) try: service_config = load_service_config_from_file(repo_path) except (ConfigParseError, ConfigValidationError): + console.warning(f"{repo} was found with an invalid config") raise except ConfigNotFoundError: # Ignore repos that don't have devservices configs diff --git a/tests/utils/test_services.py b/tests/utils/test_services.py index e17f6a31..1f5a136f 100644 --- a/tests/utils/test_services.py +++ b/tests/utils/test_services.py @@ -7,6 +7,7 @@ import pytest from devservices.configs.service_config import ServiceConfig +from devservices.exceptions import ConfigParseError from devservices.exceptions import ServiceNotFoundError from devservices.utils.services import find_matching_service from devservices.utils.services import get_local_services @@ -32,13 +33,11 @@ def test_get_local_services_with_invalid_config( mock_repo_path = mock_code_root / "example" create_mock_git_repo("invalid_repo", mock_repo_path) - local_services = get_local_services(str(mock_code_root)) + with pytest.raises(ConfigParseError): + local_services = get_local_services(str(mock_code_root)) + assert not local_services captured = capsys.readouterr() - assert not local_services - assert ( - "example was found with an invalid config: Error parsing config file:" - in captured.out - ) + assert "example was found with an invalid config" in captured.out def test_get_local_services_with_valid_config(tmp_path: Path) -> None: From eccadb2505e3cea9351a115723fd773815802cd3 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 13 May 2025 12:09:20 -0700 Subject: [PATCH 03/23] add supervisor config validation --- devservices/configs/service_config.py | 23 +++++++++- tests/configs/test_service_config.py | 64 +++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/devservices/configs/service_config.py b/devservices/configs/service_config.py index 62e459b6..1cdfde54 100644 --- a/devservices/configs/service_config.py +++ b/devservices/configs/service_config.py @@ -5,12 +5,15 @@ from dataclasses import fields import yaml +from supervisor.options import ServerOptions from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEVSERVICES_DIR_NAME +from devservices.constants import PROGRAMS_CONF_FILE_NAME from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import ConfigParseError from devservices.exceptions import ConfigValidationError +from devservices.utils.supervisor import SupervisorManager VALID_VERSIONS = [0.1] @@ -86,6 +89,10 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: docker_compose_services = config.get("services", {}).keys() + supervisor_programs = load_supervisor_programs_from_file( + repo_path, service_config_data.get("service_name") + ) + valid_dependency_keys = {field.name for field in fields(Dependency)} dependencies = {} @@ -113,9 +120,10 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: if ( dependency.remote is None and dependency_name not in docker_compose_services + and dependency_name not in supervisor_programs ): raise ConfigValidationError( - f"Dependency '{dependency_name}' is not remote but is not defined in docker-compose services" + f"Dependency '{dependency_name}' is not remote but is not defined in docker-compose services or programs file" ) service_config = ServiceConfig( @@ -126,3 +134,16 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: ) return service_config + + +def load_supervisor_programs_from_file(repo_path: str, service_name: str) -> set[str]: + programs_config_path = os.path.join( + repo_path, DEVSERVICES_DIR_NAME, PROGRAMS_CONF_FILE_NAME + ) + if not os.path.exists(programs_config_path): + return set() + manager = SupervisorManager(programs_config_path, service_name=service_name) + opts = ServerOptions() + opts.configfile = manager.config_file_path + opts.process_config() + return set([program.name for program in opts.process_group_configs]) diff --git a/tests/configs/test_service_config.py b/tests/configs/test_service_config.py index e6797630..e63ff9f6 100644 --- a/tests/configs/test_service_config.py +++ b/tests/configs/test_service_config.py @@ -10,6 +10,7 @@ from devservices.exceptions import ConfigParseError from devservices.exceptions import ConfigValidationError from testing.utils import create_config_file +from testing.utils import create_programs_conf_file @pytest.mark.parametrize( @@ -412,3 +413,66 @@ def test_load_service_config_from_file_invalid_yaml_tag(tmp_path: Path) -> None: str(e.value) == f"Error parsing config file: could not determine a constructor for the tag 'tag:yaml.org,2002:invalid_tag'\n in \"{tmp_path / 'devservices' / 'config.yml'}\", line 7, column 19" ) + + +def test_load_service_config_from_file_no_programs_file(tmp_path: Path) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "example-dependency": { + "description": "Example dependency", + }, + "example-program": { + "description": "Example program", + }, + }, + "modes": {"default": ["example-dependency", "example-program"]}, + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, + } + create_config_file(tmp_path, config) + + with pytest.raises(ConfigValidationError) as e: + load_service_config_from_file(str(tmp_path)) + assert ( + str(e.value) + == "Dependency 'example-program' is not remote but is not defined in docker-compose services or programs file" + ) + + +def test_load_service_config_from_file_valid_programs_file(tmp_path: Path) -> None: + service_config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "example-dependency": { + "description": "Example dependency", + }, + "example-program": { + "description": "Example program", + }, + }, + "modes": {"default": ["example-dependency", "example-program"]}, + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, + } + create_config_file(tmp_path, service_config) + + programs_config = """[program:example-program] +command=echo "Hello, World!" +autostart=true +""" + create_programs_conf_file(tmp_path, programs_config) + + load_service_config_from_file(str(tmp_path)) From b1e2dc70e7f8527622be8a8b4ec4406029e98f07 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 13 May 2025 12:13:06 -0700 Subject: [PATCH 04/23] add two more tests --- tests/configs/test_service_config.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/configs/test_service_config.py b/tests/configs/test_service_config.py index e63ff9f6..8d473de7 100644 --- a/tests/configs/test_service_config.py +++ b/tests/configs/test_service_config.py @@ -6,6 +6,7 @@ import pytest from devservices.configs.service_config import load_service_config_from_file +from devservices.configs.service_config import load_supervisor_programs_from_file from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import ConfigParseError from devservices.exceptions import ConfigValidationError @@ -476,3 +477,19 @@ def test_load_service_config_from_file_valid_programs_file(tmp_path: Path) -> No create_programs_conf_file(tmp_path, programs_config) load_service_config_from_file(str(tmp_path)) + + +def test_load_supervisor_programs_from_file_no_programs_file(tmp_path: Path) -> None: + programs = load_supervisor_programs_from_file(str(tmp_path), "example-service") + assert programs == set() + + +def test_load_supervisor_programs_from_file_valid_programs_file(tmp_path: Path) -> None: + programs_config = """[program:example-program] +command=echo "Hello, World!" +autostart=true +""" + create_programs_conf_file(tmp_path, programs_config) + + programs = load_supervisor_programs_from_file(str(tmp_path), "example-service") + assert programs == {"example-program"} From d9c5fc824ab64fb050c3ae3208ec06db6ff8f771 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 13 May 2025 14:06:01 -0700 Subject: [PATCH 05/23] change structure of dependeny to also pass in type --- devservices/configs/service_config.py | 28 ++++++++++++++++++------- tests/configs/test_service_config.py | 30 +++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/devservices/configs/service_config.py b/devservices/configs/service_config.py index 1cdfde54..b45b73a6 100644 --- a/devservices/configs/service_config.py +++ b/devservices/configs/service_config.py @@ -3,6 +3,7 @@ import os from dataclasses import dataclass from dataclasses import fields +from enum import Enum import yaml from supervisor.options import ServerOptions @@ -18,6 +19,11 @@ VALID_VERSIONS = [0.1] +class DependencyType(Enum): + DOCKER_COMPOSE = "docker-compose" + SUPERVISOR = "supervisor" + + @dataclass class RemoteConfig: repo_name: str @@ -30,6 +36,7 @@ class RemoteConfig: class Dependency: description: str remote: RemoteConfig | None = None + dependency_type: str | None = None @dataclass @@ -117,14 +124,19 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: # Validate that all non-remote dependencies are defined in docker-compose services for dependency_name, dependency in dependencies.items(): - if ( - dependency.remote is None - and dependency_name not in docker_compose_services - and dependency_name not in supervisor_programs - ): - raise ConfigValidationError( - f"Dependency '{dependency_name}' is not remote but is not defined in docker-compose services or programs file" - ) + if dependency.remote is None: + if dependency_name in supervisor_programs: + dependencies[ + dependency_name + ].dependency_type = DependencyType.SUPERVISOR.value + elif dependency_name in docker_compose_services: + dependencies[ + dependency_name + ].dependency_type = DependencyType.DOCKER_COMPOSE.value + else: + raise ConfigValidationError( + f"Dependency '{dependency_name}' is not remote but is not defined in docker-compose services or programs file" + ) service_config = ServiceConfig( version=service_config_data.get("version"), diff --git a/tests/configs/test_service_config.py b/tests/configs/test_service_config.py index 8d473de7..1b3f326e 100644 --- a/tests/configs/test_service_config.py +++ b/tests/configs/test_service_config.py @@ -5,6 +5,7 @@ import pytest +from devservices.configs.service_config import DependencyType from devservices.configs.service_config import load_service_config_from_file from devservices.configs.service_config import load_supervisor_programs_from_file from devservices.exceptions import ConfigNotFoundError @@ -15,12 +16,15 @@ @pytest.mark.parametrize( - "service_name, dependencies, modes", + "service_name, dependencies, modes, dependency_types", [ ( "example-service", {"example-dependency": {"description": "Example dependency"}}, {"default": ["example-dependency"]}, + { + "example-dependency": DependencyType.DOCKER_COMPOSE.value, + }, ), ( "example-service", @@ -39,6 +43,10 @@ }, }, {"default": ["example-dependency-1", "example-dependency-2"]}, + { + "example-dependency-1": None, + "example-dependency-2": DependencyType.DOCKER_COMPOSE.value, + }, ), ( "example-service", @@ -57,6 +65,10 @@ }, }, {"default": ["example-dependency-1"], "custom": ["example-dependency-2"]}, + { + "example-dependency-1": None, + "example-dependency-2": DependencyType.DOCKER_COMPOSE.value, + }, ), ], ) @@ -65,6 +77,7 @@ def test_load_service_config_from_file( service_name: str, dependencies: dict[str, dict[str, object]], modes: dict[str, list[str]], + dependency_types: dict[str, DependencyType], ) -> None: config = { "x-sentry-service-config": { @@ -90,6 +103,7 @@ def test_load_service_config_from_file( key: { "description": value["description"], "remote": value.get("remote"), + "dependency_type": dependency_types[key], } for key, value in dependencies.items() }, @@ -448,7 +462,7 @@ def test_load_service_config_from_file_no_programs_file(tmp_path: Path) -> None: def test_load_service_config_from_file_valid_programs_file(tmp_path: Path) -> None: - service_config = { + devservices_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "example-service", @@ -468,7 +482,7 @@ def test_load_service_config_from_file_valid_programs_file(tmp_path: Path) -> No } }, } - create_config_file(tmp_path, service_config) + create_config_file(tmp_path, devservices_config) programs_config = """[program:example-program] command=echo "Hello, World!" @@ -476,7 +490,15 @@ def test_load_service_config_from_file_valid_programs_file(tmp_path: Path) -> No """ create_programs_conf_file(tmp_path, programs_config) - load_service_config_from_file(str(tmp_path)) + service_config = load_service_config_from_file(str(tmp_path)) + assert ( + service_config.dependencies["example-program"].dependency_type + == DependencyType.SUPERVISOR.value + ) + assert ( + service_config.dependencies["example-dependency"].dependency_type + == DependencyType.DOCKER_COMPOSE.value + ) def test_load_supervisor_programs_from_file_no_programs_file(tmp_path: Path) -> None: From 7e4beff67936ee582a2afc569115fa49d9b27644 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 13 May 2025 14:30:10 -0700 Subject: [PATCH 06/23] fix more stuff --- devservices/configs/service_config.py | 46 ++++++++++++--------------- devservices/constants.py | 7 ++++ devservices/utils/dependencies.py | 16 ++++------ tests/configs/test_service_config.py | 16 +++++----- 4 files changed, 42 insertions(+), 43 deletions(-) diff --git a/devservices/configs/service_config.py b/devservices/configs/service_config.py index b45b73a6..bb91a7bc 100644 --- a/devservices/configs/service_config.py +++ b/devservices/configs/service_config.py @@ -3,12 +3,12 @@ import os from dataclasses import dataclass from dataclasses import fields -from enum import Enum import yaml from supervisor.options import ServerOptions from devservices.constants import CONFIG_FILE_NAME +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME from devservices.constants import PROGRAMS_CONF_FILE_NAME from devservices.exceptions import ConfigNotFoundError @@ -19,11 +19,6 @@ VALID_VERSIONS = [0.1] -class DependencyType(Enum): - DOCKER_COMPOSE = "docker-compose" - SUPERVISOR = "supervisor" - - @dataclass class RemoteConfig: repo_name: str @@ -35,8 +30,8 @@ class RemoteConfig: @dataclass class Dependency: description: str + dependency_type: DependencyType remote: RemoteConfig | None = None - dependency_type: str | None = None @dataclass @@ -111,33 +106,32 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: raise ConfigParseError( f"Unexpected key(s) in dependency '{key}': {unexpected_keys}" ) + if value.get("remote") is None: + if key in supervisor_programs: + dependency_type = DependencyType.SUPERVISOR + elif key in docker_compose_services: + dependency_type = DependencyType.COMPOSE + else: + raise ConfigValidationError( + f"Dependency '{key}' is not remote but is not defined in docker-compose services or programs file" + ) + else: + dependency_type = DependencyType.SERVICE + dependencies[key] = Dependency( description=value.get("description"), - remote=RemoteConfig(**value.get("remote")) - if "remote" in value - else None, + remote=( + RemoteConfig(**value.get("remote")) + if "remote" in value + else None + ), + dependency_type=dependency_type, ) except TypeError as type_error: raise ConfigParseError( f"Error parsing service dependencies: {type_error}" ) from type_error - # Validate that all non-remote dependencies are defined in docker-compose services - for dependency_name, dependency in dependencies.items(): - if dependency.remote is None: - if dependency_name in supervisor_programs: - dependencies[ - dependency_name - ].dependency_type = DependencyType.SUPERVISOR.value - elif dependency_name in docker_compose_services: - dependencies[ - dependency_name - ].dependency_type = DependencyType.DOCKER_COMPOSE.value - else: - raise ConfigValidationError( - f"Dependency '{dependency_name}' is not remote but is not defined in docker-compose services or programs file" - ) - service_config = ServiceConfig( version=service_config_data.get("version"), service_name=service_config_data.get("service_name"), diff --git a/devservices/constants.py b/devservices/constants.py index 80e8433b..ce6b96ed 100644 --- a/devservices/constants.py +++ b/devservices/constants.py @@ -2,6 +2,7 @@ import os from datetime import timedelta +from enum import Enum class Color: @@ -15,6 +16,12 @@ class Color: RESET = "\033[0m" +class DependencyType(str, Enum): + SERVICE = "service" + COMPOSE = "compose" + SUPERVISOR = "supervisor" + + MINIMUM_DOCKER_COMPOSE_VERSION = "2.29.7" DEVSERVICES_DIR_NAME = "devservices" CONFIG_FILE_NAME = "config.yml" diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py index 018fe714..dcae655f 100644 --- a/devservices/utils/dependencies.py +++ b/devservices/utils/dependencies.py @@ -10,7 +10,6 @@ from concurrent.futures import as_completed from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass -from enum import Enum from typing import TextIO from typing import TypeGuard @@ -24,6 +23,7 @@ from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION from devservices.constants import DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DIR_NAME from devservices.constants import LOGGER_NAME @@ -55,11 +55,6 @@ ] -class DependencyType(str, Enum): - SERVICE = "service" - COMPOSE = "compose" - - @dataclass(frozen=True, eq=True) class DependencyNode: name: str @@ -751,6 +746,7 @@ def _construct_dependency_graph( for mode in modes: service_mode_dependencies.update(service_config.modes.get(mode, [])) for dependency_name, dependency in service_config.dependencies.items(): + print(dependency_name, dependency) # Skip the dependency if it's not in the modes (since it may not be installed and we don't care about it) if dependency_name not in service_mode_dependencies: continue @@ -761,9 +757,11 @@ def _construct_dependency_graph( ), DependencyNode( name=dependency_name, - dependency_type=DependencyType.SERVICE - if _has_remote_config(dependency.remote) - else DependencyType.COMPOSE, + dependency_type=( + DependencyType.SERVICE + if _has_remote_config(dependency.remote) + else dependency.dependency_type + ), ), ) if _has_remote_config(dependency.remote): diff --git a/tests/configs/test_service_config.py b/tests/configs/test_service_config.py index 1b3f326e..7809e0b3 100644 --- a/tests/configs/test_service_config.py +++ b/tests/configs/test_service_config.py @@ -5,9 +5,9 @@ import pytest -from devservices.configs.service_config import DependencyType from devservices.configs.service_config import load_service_config_from_file from devservices.configs.service_config import load_supervisor_programs_from_file +from devservices.constants import DependencyType from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import ConfigParseError from devservices.exceptions import ConfigValidationError @@ -23,7 +23,7 @@ {"example-dependency": {"description": "Example dependency"}}, {"default": ["example-dependency"]}, { - "example-dependency": DependencyType.DOCKER_COMPOSE.value, + "example-dependency": DependencyType.COMPOSE, }, ), ( @@ -44,8 +44,8 @@ }, {"default": ["example-dependency-1", "example-dependency-2"]}, { - "example-dependency-1": None, - "example-dependency-2": DependencyType.DOCKER_COMPOSE.value, + "example-dependency-1": DependencyType.SERVICE, + "example-dependency-2": DependencyType.COMPOSE, }, ), ( @@ -66,8 +66,8 @@ }, {"default": ["example-dependency-1"], "custom": ["example-dependency-2"]}, { - "example-dependency-1": None, - "example-dependency-2": DependencyType.DOCKER_COMPOSE.value, + "example-dependency-1": DependencyType.SERVICE, + "example-dependency-2": DependencyType.COMPOSE, }, ), ], @@ -493,11 +493,11 @@ def test_load_service_config_from_file_valid_programs_file(tmp_path: Path) -> No service_config = load_service_config_from_file(str(tmp_path)) assert ( service_config.dependencies["example-program"].dependency_type - == DependencyType.SUPERVISOR.value + == DependencyType.SUPERVISOR ) assert ( service_config.dependencies["example-dependency"].dependency_type - == DependencyType.DOCKER_COMPOSE.value + == DependencyType.COMPOSE ) From eb39a558eef1403f8c7bb617b66f0d38c402efcd Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 13 May 2025 14:30:52 -0700 Subject: [PATCH 07/23] remove print statement --- devservices/utils/dependencies.py | 1 - 1 file changed, 1 deletion(-) diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py index dcae655f..bc1f01d2 100644 --- a/devservices/utils/dependencies.py +++ b/devservices/utils/dependencies.py @@ -746,7 +746,6 @@ def _construct_dependency_graph( for mode in modes: service_mode_dependencies.update(service_config.modes.get(mode, [])) for dependency_name, dependency in service_config.dependencies.items(): - print(dependency_name, dependency) # Skip the dependency if it's not in the modes (since it may not be installed and we don't care about it) if dependency_name not in service_mode_dependencies: continue From 6d226757dcb73300cac4b9c01c0c631b29ffffb0 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 13 May 2025 14:42:34 -0700 Subject: [PATCH 08/23] fix tests --- tests/utils/test_dependencies.py | 38 +++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/tests/utils/test_dependencies.py b/tests/utils/test_dependencies.py index f5ae14ae..af229246 100644 --- a/tests/utils/test_dependencies.py +++ b/tests/utils/test_dependencies.py @@ -15,6 +15,7 @@ from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION from devservices.constants import DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME from devservices.exceptions import DependencyError from devservices.exceptions import DependencyNotInstalledError @@ -23,7 +24,6 @@ from devservices.exceptions import ModeDoesNotExistError from devservices.utils.dependencies import construct_dependency_graph from devservices.utils.dependencies import DependencyNode -from devservices.utils.dependencies import DependencyType from devservices.utils.dependencies import get_installed_remote_dependencies from devservices.utils.dependencies import get_non_shared_remote_dependencies from devservices.utils.dependencies import GitConfigManager @@ -203,6 +203,7 @@ def test_verify_local_dependencies_no_remote_dependencies(tmp_path: Path) -> Non ): dependency = Dependency( description="Test dependency", + dependency_type=DependencyType.COMPOSE, ) assert verify_local_dependencies([dependency]) @@ -221,6 +222,7 @@ def test_verify_local_dependencies_with_remote_dependencies(tmp_path: Path) -> N dependency = Dependency( description="Test dependency", remote=remote_config, + dependency_type=DependencyType.COMPOSE, ) assert not verify_local_dependencies([dependency]) @@ -255,6 +257,7 @@ def test_get_installed_remote_dependencies_single_dep_not_installed( branch="main", repo_link=f"file://{tmp_path / 'test-repo'}", ), + dependency_type=DependencyType.SERVICE, ) with pytest.raises(DependencyNotInstalledError): get_installed_remote_dependencies(dependencies=[mock_dependency]) @@ -273,6 +276,7 @@ def test_get_installed_remote_dependencies_single_dep_installed(tmp_path: Path) branch="main", repo_link=f"file://{tmp_path / 'test-repo'}", ), + dependency_type=DependencyType.SERVICE, ) installed_remote_dependencies_initial = install_dependencies([mock_dependency]) installed_remote_dependencies = get_installed_remote_dependencies( @@ -1963,6 +1967,7 @@ def test_install_dependencies_nested_dependency_file_contention(tmp_path: Path) branch="main", repo_link=f"file://{repo_a_path}", ), + dependency_type=DependencyType.SERVICE, ) repo_b_dependency = Dependency( description="repo b", @@ -1971,6 +1976,7 @@ def test_install_dependencies_nested_dependency_file_contention(tmp_path: Path) branch="main", repo_link=f"file://{repo_b_path}", ), + dependency_type=DependencyType.SERVICE, ) dependencies = [repo_a_dependency, repo_b_dependency] @@ -2080,6 +2086,7 @@ def test_get_non_shared_remote_dependencies_no_shared_dependencies( repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ) }, modes={"default": ["dependency-1"]}, @@ -2136,6 +2143,7 @@ def test_get_non_shared_remote_dependencies_no_shared_dependencies( repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ) }, modes={"default": ["dependency-1"]}, @@ -2171,6 +2179,7 @@ def test_get_non_shared_remote_dependencies_shared_dependencies( repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ) }, modes={"default": ["dependency-1"]}, @@ -2200,6 +2209,7 @@ def test_get_non_shared_remote_dependencies_shared_dependencies( repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ) ] ) @@ -2234,6 +2244,7 @@ def test_get_non_shared_remote_dependencies_shared_dependencies( repo_link="file://path/to/dependency-3", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "dependency-4": Dependency( description="dependency-4", @@ -2242,6 +2253,7 @@ def test_get_non_shared_remote_dependencies_shared_dependencies( repo_link="file://path/to/dependency-4", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["dependency-3"], "other": ["dependency-4"]}, @@ -2278,6 +2290,7 @@ def test_get_non_shared_remote_dependencies_nested_shared_dependencies( repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "dependency-2": Dependency( description="dependency-2", @@ -2286,6 +2299,7 @@ def test_get_non_shared_remote_dependencies_nested_shared_dependencies( repo_link="file://path/to/dependency-2", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["dependency-1", "dependency-2"]}, @@ -2327,6 +2341,7 @@ def test_get_non_shared_remote_dependencies_nested_shared_dependencies( repo_link="file://path/to/dependency-3", branch="main", ), + dependency_type=DependencyType.SERVICE, ) ] ) @@ -2352,6 +2367,7 @@ def test_get_non_shared_remote_dependencies_nested_shared_dependencies( repo_link="file://path/to/dependency-3", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["dependency-3"]}, @@ -2388,6 +2404,7 @@ def test_get_non_shared_remote_dependencies_with_local_runtime_dependency( repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "service-2": Dependency( description="service-2", @@ -2396,6 +2413,7 @@ def test_get_non_shared_remote_dependencies_with_local_runtime_dependency( repo_link="file://path/to/service-2", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["dependency-1", "service-2"]}, @@ -2455,6 +2473,7 @@ def test_get_non_shared_remote_dependencies_with_local_runtime_dependency( repo_link="file://path/to/dependency-3", branch="main", ), + dependency_type=DependencyType.SERVICE, ) ] ) @@ -2478,6 +2497,7 @@ def test_install_and_verify_dependencies_simple( repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "dependency-2": Dependency( description="dependency-2", @@ -2486,6 +2506,7 @@ def test_install_and_verify_dependencies_simple( repo_link="file://path/to/dependency-2", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["dependency-1", "dependency-2"]}, @@ -2519,6 +2540,7 @@ def test_install_and_verify_dependencies_mode_simple( repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "dependency-2": Dependency( description="dependency-2", @@ -2527,6 +2549,7 @@ def test_install_and_verify_dependencies_mode_simple( repo_link="file://path/to/dependency-2", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={ @@ -2557,6 +2580,7 @@ def test_install_and_verify_dependencies_mode_does_not_exist(tmp_path: Path) -> repo_link="file://path/to/dependency-1", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "dependency-2": Dependency( description="dependency-2", @@ -2565,6 +2589,7 @@ def test_install_and_verify_dependencies_mode_does_not_exist(tmp_path: Path) -> repo_link="file://path/to/dependency-2", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["dependency-1", "dependency-2"]}, @@ -2615,6 +2640,7 @@ def test_construct_dependency_graph_simple( repo_link=f"file://{dependency_service_repo_path}", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={ @@ -2729,9 +2755,11 @@ def test_construct_dependency_graph_one_nested_dependency( repo_link=f"file://{parent_service_repo_path}", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "grandparent-service": Dependency( description="grandparent-service", + dependency_type=DependencyType.COMPOSE, ), }, modes={ @@ -2877,9 +2905,11 @@ def test_construct_dependency_graph_shared_dependency( repo_link=f"file://{parent_service_repo_path}", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "grandparent-service": Dependency( description="grandparent-service", + dependency_type=DependencyType.COMPOSE, ), "child-service": Dependency( description="child-service", @@ -2888,6 +2918,7 @@ def test_construct_dependency_graph_shared_dependency( repo_link=f"file://{child_service_repo_path}", branch="main", ), + dependency_type=DependencyType.SERVICE, ), }, modes={ @@ -3037,9 +3068,11 @@ def test_construct_dependency_graph_non_self_reference( repo_link=f"file://{parent_service_repo_path}", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "grandparent-service-container": Dependency( description="grandparent-service-container", + dependency_type=DependencyType.COMPOSE, ), }, modes={ @@ -3252,6 +3285,7 @@ def test_construct_dependency_graph_complex( repo_link=f"file://{child_service_repo_path}", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "grandparent-service": Dependency( description="grandparent-service", @@ -3260,9 +3294,11 @@ def test_construct_dependency_graph_complex( repo_link=f"file://{grandparent_service_repo_path}", branch="main", ), + dependency_type=DependencyType.SERVICE, ), "complex-service": Dependency( description="complex-service", + dependency_type=DependencyType.COMPOSE, ), }, modes={ From 03b77d411cf67368dde96445ffaeaf56ce1e06e8 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 13 May 2025 15:32:06 -0700 Subject: [PATCH 09/23] actually fix tests --- devservices/commands/down.py | 2 +- devservices/commands/reset.py | 2 +- devservices/commands/status.py | 2 +- devservices/commands/up.py | 2 +- tests/commands/test_down.py | 6 +++ tests/commands/test_list_dependencies.py | 9 ++++- tests/commands/test_logs.py | 17 +++++++-- tests/commands/test_status.py | 22 ++++++++--- tests/commands/test_toggle.py | 48 +++++++++++++++++++++--- 9 files changed, 90 insertions(+), 20 deletions(-) diff --git a/devservices/commands/down.py b/devservices/commands/down.py index 8d34134c..0836ecb6 100644 --- a/devservices/commands/down.py +++ b/devservices/commands/down.py @@ -11,6 +11,7 @@ from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY from devservices.constants import DEVSERVICES_DIR_NAME @@ -23,7 +24,6 @@ from devservices.utils.console import Status from devservices.utils.dependencies import construct_dependency_graph from devservices.utils.dependencies import DependencyNode -from devservices.utils.dependencies import DependencyType from devservices.utils.dependencies import get_non_shared_remote_dependencies from devservices.utils.dependencies import install_and_verify_dependencies from devservices.utils.dependencies import InstalledRemoteDependency diff --git a/devservices/commands/reset.py b/devservices/commands/reset.py index 1e936c5f..f0c909a0 100644 --- a/devservices/commands/reset.py +++ b/devservices/commands/reset.py @@ -7,6 +7,7 @@ from sentry_sdk import capture_exception from devservices.commands.down import down +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_ORCHESTRATOR_LABEL from devservices.exceptions import DockerDaemonNotRunningError from devservices.exceptions import DockerError @@ -14,7 +15,6 @@ from devservices.utils.console import Status from devservices.utils.dependencies import construct_dependency_graph from devservices.utils.dependencies import DependencyNode -from devservices.utils.dependencies import DependencyType from devservices.utils.docker import get_matching_containers from devservices.utils.docker import get_volumes_for_containers from devservices.utils.docker import remove_docker_resources diff --git a/devservices/commands/status.py b/devservices/commands/status.py index f04f4f58..59db155b 100644 --- a/devservices/commands/status.py +++ b/devservices/commands/status.py @@ -15,6 +15,7 @@ from devservices.constants import Color from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY from devservices.constants import DEVSERVICES_DIR_NAME @@ -27,7 +28,6 @@ from devservices.utils.dependencies import construct_dependency_graph from devservices.utils.dependencies import DependencyGraph from devservices.utils.dependencies import DependencyNode -from devservices.utils.dependencies import DependencyType from devservices.utils.dependencies import install_and_verify_dependencies from devservices.utils.dependencies import InstalledRemoteDependency from devservices.utils.docker_compose import get_docker_compose_commands_to_run diff --git a/devservices/commands/up.py b/devservices/commands/up.py index 0d3cb765..23a0014a 100644 --- a/devservices/commands/up.py +++ b/devservices/commands/up.py @@ -11,6 +11,7 @@ from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY from devservices.constants import DEVSERVICES_DIR_NAME @@ -25,7 +26,6 @@ from devservices.utils.console import Status from devservices.utils.dependencies import construct_dependency_graph from devservices.utils.dependencies import DependencyNode -from devservices.utils.dependencies import DependencyType from devservices.utils.dependencies import install_and_verify_dependencies from devservices.utils.dependencies import InstalledRemoteDependency from devservices.utils.docker import check_all_containers_healthy diff --git a/tests/commands/test_down.py b/tests/commands/test_down.py index 36b7ee67..125c205a 100644 --- a/tests/commands/test_down.py +++ b/tests/commands/test_down.py @@ -13,6 +13,7 @@ from devservices.configs.service_config import RemoteConfig from devservices.configs.service_config import ServiceConfig from devservices.constants import CONFIG_FILE_NAME +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME from devservices.exceptions import ConfigError from devservices.exceptions import ServiceNotFoundError @@ -632,6 +633,7 @@ def test_down_does_not_stop_service_being_used_by_another_service( dependencies={ "redis": Dependency( description="Redis", + dependency_type=DependencyType.SERVICE, remote=RemoteConfig( repo_name="redis", repo_link=f"file://{redis_repo_path}", @@ -641,6 +643,7 @@ def test_down_does_not_stop_service_being_used_by_another_service( ), "example-service": Dependency( description="Example service", + dependency_type=DependencyType.SERVICE, remote=RemoteConfig( repo_name="example-service", repo_link=f"file://{example_repo_path}", @@ -814,6 +817,7 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service( dependencies={ "parent-service": Dependency( description="Parent service", + dependency_type=DependencyType.SERVICE, remote=RemoteConfig( repo_name="parent-service", repo_link=f"file://{parent_repo_path}", @@ -1094,6 +1098,7 @@ def test_down_local_service_with_dependent_service_running( dependencies={ "redis": Dependency( description="Redis", + dependency_type=DependencyType.SERVICE, remote=RemoteConfig( repo_name="redis", repo_link=f"file://{redis_repo_path}", @@ -1103,6 +1108,7 @@ def test_down_local_service_with_dependent_service_running( ), "local-runtime-service": Dependency( description="Local runtime service", + dependency_type=DependencyType.SERVICE, remote=RemoteConfig( repo_name="local-runtime-service", repo_link=f"file://{local_runtime_repo_path}", diff --git a/tests/commands/test_list_dependencies.py b/tests/commands/test_list_dependencies.py index d866d6a1..84e247fd 100644 --- a/tests/commands/test_list_dependencies.py +++ b/tests/commands/test_list_dependencies.py @@ -10,6 +10,7 @@ from devservices.commands.list_dependencies import list_dependencies from devservices.configs.service_config import Dependency from devservices.configs.service_config import ServiceConfig +from devservices.constants import DependencyType from devservices.exceptions import ConfigValidationError from devservices.exceptions import ServiceNotFoundError from devservices.utils.services import Service @@ -115,8 +116,12 @@ def test_list_dependencies_with_dependencies( version=0.1, service_name="test-service", dependencies={ - "redis": Dependency(description="Redis"), - "postgres": Dependency(description="Postgres"), + "redis": Dependency( + description="Redis", dependency_type=DependencyType.COMPOSE + ), + "postgres": Dependency( + description="Postgres", dependency_type=DependencyType.COMPOSE + ), }, modes={"default": ["redis", "postgres"]}, ), diff --git a/tests/commands/test_logs.py b/tests/commands/test_logs.py index b5418ef0..9eda8ad5 100644 --- a/tests/commands/test_logs.py +++ b/tests/commands/test_logs.py @@ -12,6 +12,7 @@ from devservices.configs.service_config import Dependency from devservices.configs.service_config import ServiceConfig from devservices.constants import CONFIG_FILE_NAME +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME from devservices.exceptions import ConfigError from devservices.exceptions import ServiceNotFoundError @@ -42,8 +43,12 @@ def test_logs_no_specified_service_not_running( version=0.1, service_name="example-service", dependencies={ - "redis": Dependency(description="Redis"), - "clickhouse": Dependency(description="Clickhouse"), + "redis": Dependency( + description="Redis", dependency_type=DependencyType.COMPOSE + ), + "clickhouse": Dependency( + description="Clickhouse", dependency_type=DependencyType.COMPOSE + ), }, modes={"default": ["redis", "clickhouse"]}, ), @@ -98,8 +103,12 @@ def test_logs_no_specified_service_success( version=0.1, service_name="example-service", dependencies={ - "redis": Dependency(description="Redis"), - "clickhouse": Dependency(description="Clickhouse"), + "redis": Dependency( + description="Redis", dependency_type=DependencyType.COMPOSE + ), + "clickhouse": Dependency( + description="Clickhouse", dependency_type=DependencyType.COMPOSE + ), }, modes={"default": ["redis", "clickhouse"]}, ), diff --git a/tests/commands/test_status.py b/tests/commands/test_status.py index cf21a3c5..52a577b3 100644 --- a/tests/commands/test_status.py +++ b/tests/commands/test_status.py @@ -20,13 +20,13 @@ from devservices.configs.service_config import ServiceConfig from devservices.constants import Color from devservices.constants import CONFIG_FILE_NAME +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME from devservices.exceptions import DependencyError from devservices.exceptions import DockerComposeError from devservices.exceptions import ServiceNotFoundError from devservices.utils.dependencies import DependencyGraph from devservices.utils.dependencies import DependencyNode -from devservices.utils.dependencies import DependencyType from devservices.utils.services import Service from devservices.utils.state import State from devservices.utils.state import StateTables @@ -79,8 +79,14 @@ def test_get_status_json_results( version=0.1, service_name="test-service", dependencies={ - "redis": Dependency(description="Redis"), - "clickhouse": Dependency(description="Clickhouse"), + "redis": Dependency( + description="Redis", + dependency_type=DependencyType.COMPOSE, + ), + "clickhouse": Dependency( + description="Clickhouse", + dependency_type=DependencyType.COMPOSE, + ), }, modes={"default": ["redis", "clickhouse"], "test": ["redis"]}, ), @@ -541,8 +547,14 @@ def test_handle_started_service( version=0.1, service_name="test-service", dependencies={ - "redis": Dependency(description="Redis"), - "clickhouse": Dependency(description="Clickhouse"), + "redis": Dependency( + description="Redis", + dependency_type=DependencyType.COMPOSE, + ), + "clickhouse": Dependency( + description="Clickhouse", + dependency_type=DependencyType.COMPOSE, + ), }, modes={"default": ["redis", "clickhouse"], "test": ["redis"]}, ), diff --git a/tests/commands/test_toggle.py b/tests/commands/test_toggle.py index f397ee61..f15752f2 100644 --- a/tests/commands/test_toggle.py +++ b/tests/commands/test_toggle.py @@ -17,6 +17,7 @@ from devservices.configs.service_config import RemoteConfig from devservices.configs.service_config import ServiceConfig from devservices.constants import CONFIG_FILE_NAME +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME from devservices.exceptions import CannotToggleNonRemoteServiceError from devservices.exceptions import ConfigNotFoundError @@ -333,6 +334,7 @@ def test_toggle_dependent_service_running( branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), "example-service": Dependency( description="Example service", @@ -342,6 +344,7 @@ def test_toggle_dependent_service_running( branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["redis", "example-service"]}, @@ -416,6 +419,7 @@ def test_toggle_to_local_runtime_no_runtime_specified( "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -439,6 +443,7 @@ def test_toggle_to_local_runtime_no_runtime_specified( "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -468,6 +473,7 @@ def test_toggle_cannot_toggle_non_remote_service( "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -497,6 +503,7 @@ def test_toggle_cannot_toggle_non_remote_service( "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -536,6 +543,7 @@ def test_handle_transition_to_local_runtime_currently_running_standalone( "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -616,6 +624,7 @@ def test_handle_transition_to_local_runtime_naming_conflict( "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -640,6 +649,7 @@ def test_handle_transition_to_local_runtime_naming_conflict( "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -694,6 +704,7 @@ def test_handle_transition_to_containerized_runtime_no_dependent_services( "redis": Dependency( description="Redis", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["redis"]}, @@ -745,7 +756,11 @@ def test_handle_transition_to_containerized_runtime_with_service_running( version=0.1, service_name="redis", dependencies={ - "redis": Dependency(description="Redis", remote=None), + "redis": Dependency( + description="Redis", + remote=None, + dependency_type=DependencyType.COMPOSE, + ), }, modes={"default": ["redis"]}, ), @@ -872,6 +887,7 @@ def test_handle_transition_to_containerized_runtime_with_dependent_services( branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), "example-service": Dependency( description="Example service", @@ -881,6 +897,7 @@ def test_handle_transition_to_containerized_runtime_with_dependent_services( branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["redis", "example-service"]}, @@ -911,10 +928,12 @@ def test_handle_transition_to_containerized_runtime_with_dependent_services( branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["redis", "clickhouse"]}, @@ -1162,6 +1181,7 @@ def test_bring_down_containerized_service_install_and_verify_dependencies_failur "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -1214,6 +1234,7 @@ def test_bring_down_containerized_service_no_remote_dependencies( "clickhouse": Dependency( description="Clickhouse", remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, @@ -1230,7 +1251,11 @@ def test_bring_down_containerized_service_no_remote_dependencies( version=0.1, service_name="example-service", dependencies={ - "clickhouse": Dependency(description="Clickhouse", remote=None), + "clickhouse": Dependency( + description="Clickhouse", + remote=None, + dependency_type=DependencyType.COMPOSE, + ), }, modes={"default": ["clickhouse"]}, ), @@ -1319,7 +1344,11 @@ def test_bring_down_containerized_service_with_remote_dependency( version=0.1, service_name="example-service", dependencies={ - "clickhouse": Dependency(description="Clickhouse", remote=None), + "clickhouse": Dependency( + description="Clickhouse", + remote=None, + dependency_type=DependencyType.COMPOSE, + ), "redis": Dependency( description="Redis", remote=RemoteConfig( @@ -1328,6 +1357,7 @@ def test_bring_down_containerized_service_with_remote_dependency( branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["clickhouse", "redis"]}, @@ -1347,7 +1377,9 @@ def test_bring_down_containerized_service_with_remote_dependency( service_name="example-service", dependencies={ "clickhouse": Dependency( - description="Clickhouse", remote=None + description="Clickhouse", + remote=None, + dependency_type=DependencyType.COMPOSE, ), "redis": Dependency( description="Redis", @@ -1357,6 +1389,7 @@ def test_bring_down_containerized_service_with_remote_dependency( branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["clickhouse", "redis"]}, @@ -1410,6 +1443,7 @@ def test_bring_down_containerized_service_get_non_shared_remote_dependencies_err branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["redis"]}, @@ -1434,6 +1468,7 @@ def test_bring_down_containerized_service_get_non_shared_remote_dependencies_err branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["redis"]}, @@ -1458,6 +1493,7 @@ def test_bring_down_containerized_service_get_non_shared_remote_dependencies_err branch="main", mode="default", ), + dependency_type=DependencyType.SERVICE, ), }, modes={"default": ["redis"]}, @@ -1519,7 +1555,9 @@ def test_bring_down_containerized_service_docker_compose_error( service_name="example-service", dependencies={ "clickhouse": Dependency( - description="Clickhouse", remote=None + description="Clickhouse", + remote=None, + dependency_type=DependencyType.COMPOSE, ), }, modes={"default": ["clickhouse"]}, From c92b0c16639910ce9bf76b899a6435aa9e1e623d Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Wed, 14 May 2025 09:39:16 -0700 Subject: [PATCH 10/23] fix dependency_type --- devservices/utils/dependencies.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py index bc1f01d2..6fe0b8d8 100644 --- a/devservices/utils/dependencies.py +++ b/devservices/utils/dependencies.py @@ -755,12 +755,7 @@ def _construct_dependency_graph( dependency_type=DependencyType.SERVICE, ), DependencyNode( - name=dependency_name, - dependency_type=( - DependencyType.SERVICE - if _has_remote_config(dependency.remote) - else dependency.dependency_type - ), + name=dependency_name, dependency_type=dependency.dependency_type ), ) if _has_remote_config(dependency.remote): From 0a8c34f3fd81baf318c6e42bd94e47035f75ac86 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Thu, 15 May 2025 16:04:09 -0700 Subject: [PATCH 11/23] add supervisor program to up --- devservices/commands/up.py | 50 ++++- devservices/utils/supervisor.py | 56 ++++++ tests/commands/test_up.py | 319 ++++++++++++++++++++++++++++++++ tests/utils/test_supervisor.py | 106 ++++++++++- 4 files changed, 527 insertions(+), 4 deletions(-) diff --git a/devservices/commands/up.py b/devservices/commands/up.py index 23a0014a..bd023086 100644 --- a/devservices/commands/up.py +++ b/devservices/commands/up.py @@ -15,6 +15,7 @@ from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY from devservices.constants import DEVSERVICES_DIR_NAME +from devservices.constants import PROGRAMS_CONF_FILE_NAME from devservices.exceptions import ConfigError from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import ContainerHealthcheckFailedError @@ -22,6 +23,8 @@ from devservices.exceptions import DockerComposeError from devservices.exceptions import ModeDoesNotExistError from devservices.exceptions import ServiceNotFoundError +from devservices.exceptions import SupervisorConfigError +from devservices.exceptions import SupervisorError from devservices.utils.console import Console from devservices.utils.console import Status from devservices.utils.dependencies import construct_dependency_graph @@ -38,6 +41,7 @@ from devservices.utils.state import ServiceRuntime from devservices.utils.state import State from devservices.utils.state import StateTables +from devservices.utils.supervisor import SupervisorManager def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: @@ -151,6 +155,15 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: mode_dependencies = [ dep for dep in mode_dependencies if dep not in services_with_local_runtime ] + + supervisor_programs = [ + dep + for dep in mode_dependencies + if dep in service.config.dependencies + and service.config.dependencies[dep].dependency_type + == DependencyType.SUPERVISOR + ] + remote_dependencies = { dep for dep in remote_dependencies @@ -169,12 +182,18 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: ) status.warning(f"Continuing with service '{service.name}'") try: - _up(service, [mode], remote_dependencies, mode_dependencies, status) + bring_up_docker_compose_services( + service, [mode], remote_dependencies, mode_dependencies, status + ) except DockerComposeError as dce: capture_exception(dce, level="info") status.failure(f"Failed to start {service.name}: {dce.stderr}") exit(1) - # TODO: We should factor in healthchecks here before marking service as running + try: + bring_up_supervisor_programs(supervisor_programs, service, status) + except SupervisorError as se: + status.failure(str(se)) + exit(1) state.remove_service_entry(service.name, StateTables.STARTING_SERVICES) state.update_service_entry(service.name, mode, StateTables.STARTED_SERVICES) @@ -195,7 +214,7 @@ def _bring_up_dependency( run_cmd(cmd.full_command, current_env) -def _up( +def bring_up_docker_compose_services( service: Service, modes: list[str], remote_dependencies: set[InstalledRemoteDependency], @@ -286,6 +305,31 @@ def _up( exit(1) +def bring_up_supervisor_programs( + supervisor_programs: list[str], service: Service, status: Status +) -> None: + if len(supervisor_programs) == 0: + return + programs_config_path = os.path.join( + service.repo_path, f"{DEVSERVICES_DIR_NAME}/{PROGRAMS_CONF_FILE_NAME}" + ) + if not os.path.exists(programs_config_path): + raise SupervisorConfigError( + f"No programs.conf file found in {programs_config_path}." + ) + manager = SupervisorManager( + programs_config_path, + service_name=service.name, + ) + + status.info("Starting supervisor daemon") + manager.start_supervisor_daemon() + + for program in supervisor_programs: + status.info(f"Starting {program}") + manager.start_process(program) + + def _create_devservices_network() -> None: subprocess.run( ["docker", "network", "create", "devservices"], diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index 6d4dc39b..579929d8 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -5,6 +5,7 @@ import os import socket import subprocess +import time import xmlrpc.client from enum import IntEnum @@ -35,6 +36,19 @@ class SupervisorProcessState(IntEnum): UNKNOWN = 1000 +class SupervisorDaemonState(IntEnum): + """ + Supervisor daemon states. + + https://supervisord.org/api.html#supervisor.rpcinterface.SupervisorNamespaceRPCInterface.getState + """ + + FATAL = 2 + RUNNING = 1 + RESTARTING = 0 + SHUTDOWN = -1 + + class UnixSocketHTTPConnection(http.client.HTTPConnection): """HTTP connection over Unix sockets.""" @@ -137,9 +151,51 @@ def _is_program_running(self, program_name: str) -> bool: # If we can't get the process info, assume it's not running return False + def _wait_for_supervisor_ready( + self, timeout: int = 10, interval: float = 0.5 + ) -> None: + for _ in range(int(timeout / interval)): + try: + client = self._get_rpc_client() + state = client.supervisor.getState() + # Unfortunately supervisor is untyped, so we need to assert the types + assert isinstance(state, dict) + assert "statecode" in state + if state.get("statecode") == SupervisorDaemonState.RUNNING: + return + time.sleep(interval) + except ( + SupervisorConnectionError, + socket.error, + ConnectionRefusedError, + xmlrpc.client.Fault, + ): + time.sleep(interval) + + raise SupervisorError( + f"Supervisor didn't become ready within {timeout} seconds" + ) + def start_supervisor_daemon(self) -> None: + # Check if supervisor is already running by attempting to connect to it + try: + client = self._get_rpc_client() + client.supervisor.getState() + # Supervisor is already running, restart it since config may have changed + client.supervisor.restart() + + # Wait for supervisor to be ready after restart + self._wait_for_supervisor_ready() + return + except (SupervisorConnectionError, socket.error, ConnectionRefusedError): + # Supervisor is not running, so we need to start it + pass + try: subprocess.run(["supervisord", "-c", self.config_file_path], check=True) + + # Wait for supervisor to be ready after starting + self._wait_for_supervisor_ready() except subprocess.CalledProcessError as e: raise SupervisorError(f"Failed to start supervisor: {str(e)}") except FileNotFoundError: diff --git a/tests/commands/test_up.py b/tests/commands/test_up.py index 1274671b..d66a460b 100644 --- a/tests/commands/test_up.py +++ b/tests/commands/test_up.py @@ -8,21 +8,30 @@ import pytest +from devservices.commands.up import bring_up_supervisor_programs from devservices.commands.up import up +from devservices.configs.service_config import Dependency +from devservices.configs.service_config import ServiceConfig from devservices.constants import CONFIG_FILE_NAME +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME from devservices.constants import HEALTHCHECK_TIMEOUT +from devservices.constants import PROGRAMS_CONF_FILE_NAME from devservices.exceptions import ConfigError from devservices.exceptions import ContainerHealthcheckFailedError from devservices.exceptions import DependencyError from devservices.exceptions import DockerComposeError from devservices.exceptions import ServiceNotFoundError +from devservices.exceptions import SupervisorConfigError +from devservices.exceptions import SupervisorError from devservices.utils.docker_compose import DockerComposeCommand +from devservices.utils.services import Service from devservices.utils.state import ServiceRuntime from devservices.utils.state import State from devservices.utils.state import StateTables from testing.utils import create_config_file from testing.utils import create_mock_git_repo +from testing.utils import create_programs_conf_file from testing.utils import run_git_command @@ -2211,3 +2220,313 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta "Skipping 'local-runtime-service' as it is set to run locally" not in captured.out.strip() ), "This shouldn't be printed since other-service isn't being brought up in a mode that includes local-runtime-service" + + +@mock.patch("devservices.utils.state.State.remove_service_entry") +@mock.patch("devservices.utils.state.State.update_service_entry") +@mock.patch("devservices.commands.up._create_devservices_network") +@mock.patch("devservices.commands.up.check_all_containers_healthy") +@mock.patch( + "devservices.utils.docker_compose.get_non_remote_services", + return_value={"clickhouse", "redis"}, +) +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +def test_up_supervisor_program( + mock_start_process: mock.Mock, + mock_start_supervisor_daemon: mock.Mock, + mock_get_non_remote_services: mock.Mock, + mock_check_all_containers_healthy: mock.Mock, + mock_create_devservices_network: mock.Mock, + mock_update_service_entry: mock.Mock, + mock_remove_service_entry: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + with ( + mock.patch( + "devservices.commands.up.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + ): + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "supervisor-program": {"description": "Supervisor program"}, + }, + "modes": {"default": ["supervisor-program"]}, + }, + "services": {}, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + supervisor_program_config = """ +[program:supervisor-program] +command=echo "Hello, world!" +""" + + create_programs_conf_file(service_path, supervisor_program_config) + + args = Namespace( + service_name=None, debug=False, mode="default", exclude_local=False + ) + + up(args) + + mock_create_devservices_network.assert_called_once() + + mock_get_non_remote_services.assert_has_calls( + [ + mock.call( + f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}", + mock.ANY, + ), + mock.call( + f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}", + mock.ANY, + ), + ] + ) + + mock_update_service_entry.assert_has_calls( + [ + mock.call("example-service", "default", StateTables.STARTING_SERVICES), + mock.call("example-service", "default", StateTables.STARTED_SERVICES), + ] + ) + mock_remove_service_entry.assert_called_once_with( + "example-service", StateTables.STARTING_SERVICES + ) + mock_check_all_containers_healthy.assert_called_once() + mock_start_supervisor_daemon.assert_called_once() + mock_start_process.assert_called_once_with("supervisor-program") + captured = capsys.readouterr() + assert "Retrieving dependencies" in captured.out.strip() + assert ( + "Starting dependencies with local runtimes..." not in captured.out.strip() + ), "This shouldn't be printed since we don't have any dependencies with local runtimes" + assert "Starting 'example-service' in mode: 'default'" in captured.out.strip() + assert "Starting supervisor daemon" in captured.out.strip() + assert "Starting supervisor-program" in captured.out.strip() + + +@mock.patch("devservices.utils.state.State.remove_service_entry") +@mock.patch("devservices.utils.state.State.update_service_entry") +@mock.patch("devservices.commands.up._create_devservices_network") +@mock.patch("devservices.commands.up.check_all_containers_healthy") +@mock.patch( + "devservices.utils.docker_compose.get_non_remote_services", + return_value={"clickhouse", "redis"}, +) +@mock.patch( + "devservices.utils.supervisor.SupervisorManager.start_supervisor_daemon", + side_effect=SupervisorError("Error starting supervisor daemon"), +) +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +def test_up_supervisor_program_error( + mock_start_process: mock.Mock, + mock_start_supervisor_daemon: mock.Mock, + mock_get_non_remote_services: mock.Mock, + mock_check_all_containers_healthy: mock.Mock, + mock_create_devservices_network: mock.Mock, + mock_update_service_entry: mock.Mock, + mock_remove_service_entry: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + with ( + mock.patch( + "devservices.commands.up.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + ): + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "supervisor-program": {"description": "Supervisor program"}, + }, + "modes": {"default": ["supervisor-program"]}, + }, + "services": {}, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + supervisor_program_config = """ +[program:supervisor-program] +command=echo "Hello, world!" +""" + + create_programs_conf_file(service_path, supervisor_program_config) + + args = Namespace( + service_name=None, debug=False, mode="default", exclude_local=False + ) + + with pytest.raises(SystemExit): + up(args) + + mock_create_devservices_network.assert_called_once() + + mock_get_non_remote_services.assert_has_calls( + [ + mock.call( + f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}", + mock.ANY, + ), + mock.call( + f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}", + mock.ANY, + ), + ] + ) + + mock_update_service_entry.assert_has_calls( + [ + mock.call("example-service", "default", StateTables.STARTING_SERVICES), + ] + ) + mock_remove_service_entry.assert_not_called() + mock_check_all_containers_healthy.assert_called_once() + mock_start_supervisor_daemon.assert_called_once() + mock_start_process.assert_not_called() + captured = capsys.readouterr() + assert "Retrieving dependencies" in captured.out.strip() + assert ( + "Starting dependencies with local runtimes..." not in captured.out.strip() + ), "This shouldn't be printed since we don't have any dependencies with local runtimes" + assert "Starting 'example-service' in mode: 'default'" in captured.out.strip() + assert "Starting supervisor daemon" in captured.out.strip() + assert "Error starting supervisor daemon" in captured.out.strip() + + +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +def test_bring_up_supervisor_programs_no_programs_config( + mock_start_process: mock.Mock, + mock_start_supervisor_daemon: mock.Mock, + tmp_path: Path, +) -> None: + service_config = ServiceConfig( + version=0.1, + service_name="test-service", + dependencies={ + "supervisor-program": Dependency( + description="Supervisor program", + dependency_type=DependencyType.SUPERVISOR, + ), + }, + modes={"default": ["supervisor-program"]}, + ) + service = Service( + name="test-service", + repo_path=str(tmp_path), + config=service_config, + ) + + status = mock.MagicMock() + + with pytest.raises( + SupervisorConfigError, + match=f"No programs.conf file found in {tmp_path / DEVSERVICES_DIR_NAME / PROGRAMS_CONF_FILE_NAME}", + ): + bring_up_supervisor_programs(["supervisor-program"], service, status) + + mock_start_supervisor_daemon.assert_not_called() + mock_start_process.assert_not_called() + + +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +def test_bring_up_supervisor_programs_empty_list( + mock_start_process: mock.Mock, + mock_start_supervisor_daemon: mock.Mock, + tmp_path: Path, +) -> None: + service_config = ServiceConfig( + version=0.1, + service_name="test-service", + dependencies={ + "supervisor-program": Dependency( + description="Supervisor program", + dependency_type=DependencyType.SUPERVISOR, + ), + }, + modes={"default": ["supervisor-program"]}, + ) + service = Service( + name="test-service", + repo_path=str(tmp_path), + config=service_config, + ) + + status = mock.MagicMock() + + bring_up_supervisor_programs([], service, status) + + status.info.assert_not_called() + status.failure.assert_not_called() + + mock_start_supervisor_daemon.assert_not_called() + mock_start_process.assert_not_called() + + +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +def test_bring_up_supervisor_programs_success( + mock_start_process: mock.Mock, + mock_start_supervisor_daemon: mock.Mock, + tmp_path: Path, +) -> None: + service_config = ServiceConfig( + version=0.1, + service_name="test-service", + dependencies={ + "supervisor-program": Dependency( + description="Supervisor program", + dependency_type=DependencyType.SUPERVISOR, + ), + }, + modes={"default": ["supervisor-program"]}, + ) + service = Service( + name="test-service", + repo_path=str(tmp_path), + config=service_config, + ) + + programs_conf_path = tmp_path / DEVSERVICES_DIR_NAME / PROGRAMS_CONF_FILE_NAME + + create_programs_conf_file( + programs_conf_path, + """ +[program:supervisor-program] +command=echo "Hello, world!" +""", + ) + + status = mock.MagicMock() + + bring_up_supervisor_programs(["supervisor-program"], service, status) + + status.info.assert_has_calls( + [ + mock.call("Starting supervisor daemon"), + mock.call("Starting supervisor-program"), + ] + ) + status.failure.assert_not_called() + + mock_start_supervisor_daemon.assert_called_once() + mock_start_process.assert_called_once_with("supervisor-program") diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index b0b61a9e..e0e3e055 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -7,12 +7,14 @@ from unittest import mock import pytest +from freezegun import freeze_time from devservices.constants import DEVSERVICES_DIR_NAME from devservices.exceptions import SupervisorConfigError from devservices.exceptions import SupervisorConnectionError from devservices.exceptions import SupervisorError from devservices.exceptions import SupervisorProcessError +from devservices.utils.supervisor import SupervisorDaemonState from devservices.utils.supervisor import SupervisorManager from devservices.utils.supervisor import SupervisorProcessState from devservices.utils.supervisor import UnixSocketHTTPConnection @@ -158,15 +160,45 @@ def test_is_program_running_failure( @mock.patch("devservices.utils.supervisor.subprocess.run") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") def test_start_supervisor_daemon_success( - mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, ) -> None: + mock_rpc_client.return_value.supervisor.getState.side_effect = [ + SupervisorConnectionError("Connection refused"), + { + "statecode": SupervisorDaemonState.RUNNING, + "statename": "RUNNING", + }, + ] supervisor_manager.start_supervisor_daemon() mock_subprocess_run.assert_called_once_with( ["supervisord", "-c", supervisor_manager.config_file_path], check=True ) +@mock.patch("devservices.utils.supervisor.subprocess.run") +@mock.patch( + "devservices.utils.supervisor.xmlrpc.client.ServerProxy", + return_value=mock.MagicMock(), +) +def test_start_supervisor_daemon_already_running( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + mock_rpc_client.return_value.supervisor.getState.return_value = { + "statecode": SupervisorDaemonState.RUNNING, + "statename": "RUNNING", + } + supervisor_manager.start_supervisor_daemon() + assert mock_rpc_client.return_value.supervisor.getState.call_count == 2 + assert mock_rpc_client.return_value.supervisor.restart.call_count == 1 + assert mock_subprocess_run.call_count == 0 + + @mock.patch("devservices.utils.supervisor.subprocess.run") def test_start_supervisor_daemon_subprocess_failure( mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager @@ -404,3 +436,75 @@ def test_tail_program_logs_keyboard_interrupt( ], check=True, ) + + +@mock.patch("xmlrpc.client.ServerProxy", return_value=mock.MagicMock()) +@mock.patch("devservices.utils.supervisor.time.sleep") +def test_wait_for_supervisor_ready_success( + mock_sleep: mock.MagicMock, + mock_rpc_client: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + # Mock client that returns a running state + mock_rpc_client.return_value.supervisor.getState.return_value = { + "statename": "RUNNING", + "statecode": SupervisorDaemonState.RUNNING, + } + mock_rpc_client.return_value = supervisor_manager._get_rpc_client() + # Should not raise an exception + supervisor_manager._wait_for_supervisor_ready() + + # Should not have needed to sleep + mock_sleep.assert_not_called() + + +@mock.patch("xmlrpc.client.ServerProxy", return_value=mock.MagicMock()) +@mock.patch("devservices.utils.supervisor.time.sleep") +def test_wait_for_supervisor_ready_retries( + mock_sleep: mock.MagicMock, + mock_rpc_client: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + # First attempt fails with connection error, second has wrong state, third succeeds + call1 = SupervisorConnectionError("Connection refused") + call2 = {"statename": "RUNNING", "statecode": SupervisorDaemonState.RESTARTING} + call3 = {"statename": "RUNNING", "statecode": SupervisorDaemonState.RUNNING} + + mock_rpc_client.return_value.supervisor.getState.side_effect = [ + call1, + call2, + call3, + ] + + with freeze_time() as frozen_time: + mock_sleep.side_effect = lambda x: frozen_time.tick(x) + supervisor_manager._wait_for_supervisor_ready() + + assert mock_sleep.call_count == 2 + + assert mock_rpc_client.return_value.supervisor.getState.call_count == 3 + + +@mock.patch("xmlrpc.client.ServerProxy") +@mock.patch("devservices.utils.supervisor.time.sleep") +def test_wait_for_supervisor_ready_timeout( + mock_sleep: mock.MagicMock, + mock_rpc_client: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + mock_rpc_client.return_value.supervisor.getState.side_effect = ( + SupervisorConnectionError("Connection refused") + ) + + with ( + freeze_time() as frozen_time, + pytest.raises( + SupervisorError, match="Supervisor didn't become ready within 5 seconds" + ), + ): + mock_sleep.side_effect = lambda x: frozen_time.tick(x) + supervisor_manager._wait_for_supervisor_ready(5, 1) + + assert mock_sleep.call_count == 5 + + assert mock_rpc_client.return_value.supervisor.getState.call_count == 5 From 879295e2dee8ddefc212f099720be3a828e5c56d Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Thu, 22 May 2025 15:08:30 -0700 Subject: [PATCH 12/23] add supervisor to down --- devservices/commands/down.py | 43 ++++++ tests/commands/test_down.py | 263 +++++++++++++++++++++++++++++++++++ 2 files changed, 306 insertions(+) diff --git a/devservices/commands/down.py b/devservices/commands/down.py index 0836ecb6..163ffed2 100644 --- a/devservices/commands/down.py +++ b/devservices/commands/down.py @@ -15,11 +15,14 @@ from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY from devservices.constants import DEVSERVICES_DIR_NAME +from devservices.constants import PROGRAMS_CONF_FILE_NAME from devservices.exceptions import ConfigError from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import DependencyError from devservices.exceptions import DockerComposeError from devservices.exceptions import ServiceNotFoundError +from devservices.exceptions import SupervisorConfigError +from devservices.exceptions import SupervisorError from devservices.utils.console import Console from devservices.utils.console import Status from devservices.utils.dependencies import construct_dependency_graph @@ -35,6 +38,7 @@ from devservices.utils.state import ServiceRuntime from devservices.utils.state import State from devservices.utils.state import StateTables +from devservices.utils.supervisor import SupervisorManager def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: @@ -105,6 +109,14 @@ def down(args: Namespace) -> None: active_mode_dependencies = modes.get(active_mode, []) mode_dependencies.update(active_mode_dependencies) + supervisor_programs = [ + dep + for dep in mode_dependencies + if dep in service.config.dependencies + and service.config.dependencies[dep].dependency_type + == DependencyType.SUPERVISOR + ] + with Status( lambda: console.warning(f"Stopping {service.name}"), ) as status: @@ -129,6 +141,12 @@ def down(args: Namespace) -> None: ) exit(1) + try: + bring_down_supervisor_programs(supervisor_programs, service, status) + except SupervisorError as se: + status.failure(str(se)) + exit(1) + # Check if any service depends on the service we are trying to bring down # TODO: We should also take into account the active modes of the other services (this is not trivial to do) other_started_services = active_services.difference({service.name}) @@ -285,3 +303,28 @@ def _bring_down_dependency( for dependency in cmd.services: status.info(f"Stopping {dependency}") return run_cmd(cmd.full_command, current_env) + + +def bring_down_supervisor_programs( + supervisor_programs: list[str], service: Service, status: Status +) -> None: + if len(supervisor_programs) == 0: + return + programs_config_path = os.path.join( + service.repo_path, f"{DEVSERVICES_DIR_NAME}/{PROGRAMS_CONF_FILE_NAME}" + ) + if not os.path.exists(programs_config_path): + raise SupervisorConfigError( + f"No programs.conf file found in {programs_config_path}." + ) + manager = SupervisorManager( + programs_config_path, + service_name=service.name, + ) + + for program in supervisor_programs: + status.info(f"Stopping {program}") + manager.stop_process(program) + + status.info("Stopping supervisor daemon") + manager.stop_supervisor_daemon() diff --git a/tests/commands/test_down.py b/tests/commands/test_down.py index 125c205a..0abb9374 100644 --- a/tests/commands/test_down.py +++ b/tests/commands/test_down.py @@ -8,6 +8,7 @@ import pytest +from devservices.commands.down import bring_down_supervisor_programs from devservices.commands.down import down from devservices.configs.service_config import Dependency from devservices.configs.service_config import RemoteConfig @@ -15,8 +16,11 @@ from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME +from devservices.constants import PROGRAMS_CONF_FILE_NAME from devservices.exceptions import ConfigError from devservices.exceptions import ServiceNotFoundError +from devservices.exceptions import SupervisorConfigError +from devservices.exceptions import SupervisorError from devservices.utils.dependencies import install_and_verify_dependencies from devservices.utils.docker_compose import DockerComposeCommand from devservices.utils.services import Service @@ -25,6 +29,7 @@ from devservices.utils.state import StateTables from testing.utils import create_config_file from testing.utils import create_mock_git_repo +from testing.utils import create_programs_conf_file from testing.utils import run_git_command @@ -1334,3 +1339,261 @@ def test_down_shared_and_local_dependencies( mock.call("other-service", StateTables.STARTED_SERVICES), ] ) + + +@mock.patch("devservices.utils.state.State.remove_service_entry") +@mock.patch( + "devservices.utils.supervisor.SupervisorManager.stop_supervisor_daemon", + side_effect=SupervisorError("Error stopping supervisor daemon"), +) +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_down_supervisor_program_error( + mock_stop_process: mock.Mock, + mock_stop_supervisor_daemon: mock.Mock, + mock_remove_service_entry: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + with mock.patch( + "devservices.commands.down.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ): + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "supervisor-program": {"description": "Supervisor program"}, + }, + "modes": {"default": ["supervisor-program"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + "clickhouse": { + "image": "altinity/clickhouse-server:23.8.11.29.altinitystable" + }, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + supervisor_program_config = """ +[program:supervisor-program] +command=echo "Hello, world!" +""" + create_programs_conf_file(service_path, supervisor_program_config) + + args = Namespace(service_name=None, debug=False, exclude_local=False) + + with ( + mock.patch( + "devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state") + ), + pytest.raises(SystemExit), + ): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTING_SERVICES + ) + down(args) + + mock_remove_service_entry.assert_not_called() + + captured = capsys.readouterr() + mock_stop_process.assert_called_once_with("supervisor-program") + mock_stop_supervisor_daemon.assert_called_once() + assert "Stopping supervisor-program" in captured.out.strip() + assert "Stopping supervisor daemon" in captured.out.strip() + assert "Error stopping supervisor daemon" in captured.out.strip() + + +@mock.patch("devservices.utils.state.State.remove_service_entry") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_down_supervisor_program_success( + mock_stop_process: mock.Mock, + mock_stop_supervisor_daemon: mock.Mock, + mock_remove_service_entry: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + with mock.patch( + "devservices.commands.down.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ): + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "supervisor-program": {"description": "Supervisor program"}, + }, + "modes": {"default": ["supervisor-program"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + "clickhouse": { + "image": "altinity/clickhouse-server:23.8.11.29.altinitystable" + }, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + supervisor_program_config = """ +[program:supervisor-program] +command=echo "Hello, world!" +""" + create_programs_conf_file(service_path, supervisor_program_config) + + args = Namespace(service_name=None, debug=False, exclude_local=False) + + with ( + mock.patch( + "devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state") + ), + ): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTING_SERVICES + ) + down(args) + + mock_remove_service_entry.assert_has_calls( + [ + mock.call("example-service", StateTables.STARTING_SERVICES), + mock.call("example-service", StateTables.STARTED_SERVICES), + ] + ) + + captured = capsys.readouterr() + mock_stop_process.assert_called_once_with("supervisor-program") + mock_stop_supervisor_daemon.assert_called_once() + assert "Stopping supervisor-program" in captured.out.strip() + assert "Stopping supervisor daemon" in captured.out.strip() + assert "example-service stopped" in captured.out.strip() + + +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_bring_down_supervisor_programs_no_programs_config( + mock_stop_process: mock.Mock, + mock_stop_supervisor_daemon: mock.Mock, + tmp_path: Path, +) -> None: + service_config = ServiceConfig( + version=0.1, + service_name="test-service", + dependencies={ + "supervisor-program": Dependency( + description="Supervisor program", + dependency_type=DependencyType.SUPERVISOR, + ), + }, + modes={"default": ["supervisor-program"]}, + ) + service = Service( + name="test-service", + repo_path=str(tmp_path), + config=service_config, + ) + + status = mock.MagicMock() + + with pytest.raises( + SupervisorConfigError, + match=f"No programs.conf file found in {tmp_path / DEVSERVICES_DIR_NAME / PROGRAMS_CONF_FILE_NAME}", + ): + bring_down_supervisor_programs(["supervisor-program"], service, status) + + mock_stop_supervisor_daemon.assert_not_called() + mock_stop_process.assert_not_called() + + +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_bring_down_supervisor_programs_empty_list( + mock_stop_process: mock.Mock, + mock_stop_supervisor_daemon: mock.Mock, + tmp_path: Path, +) -> None: + service_config = ServiceConfig( + version=0.1, + service_name="test-service", + dependencies={ + "supervisor-program": Dependency( + description="Supervisor program", + dependency_type=DependencyType.SUPERVISOR, + ), + }, + modes={"default": ["supervisor-program"]}, + ) + service = Service( + name="test-service", + repo_path=str(tmp_path), + config=service_config, + ) + + status = mock.MagicMock() + + bring_down_supervisor_programs([], service, status) + + status.info.assert_not_called() + status.failure.assert_not_called() + + mock_stop_supervisor_daemon.assert_not_called() + mock_stop_process.assert_not_called() + + +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_bring_down_supervisor_programs_success( + mock_stop_process: mock.Mock, + mock_stop_supervisor_daemon: mock.Mock, + tmp_path: Path, +) -> None: + service_config = ServiceConfig( + version=0.1, + service_name="test-service", + dependencies={ + "supervisor-program": Dependency( + description="Supervisor program", + dependency_type=DependencyType.SUPERVISOR, + ), + }, + modes={"default": ["supervisor-program"]}, + ) + service = Service( + name="test-service", + repo_path=str(tmp_path), + config=service_config, + ) + + programs_conf_path = tmp_path / DEVSERVICES_DIR_NAME / PROGRAMS_CONF_FILE_NAME + + create_programs_conf_file( + programs_conf_path, + """ +[program:supervisor-program] +command=echo "Hello, world!" +""", + ) + + status = mock.MagicMock() + + bring_down_supervisor_programs(["supervisor-program"], service, status) + + status.info.assert_has_calls( + [ + mock.call("Stopping supervisor-program"), + mock.call("Stopping supervisor daemon"), + ] + ) + status.failure.assert_not_called() + + mock_stop_supervisor_daemon.assert_called_once() + mock_stop_process.assert_called_once_with("supervisor-program") From 8afa94eb9bb67bfcb13ed0728769d6da5adfe39f Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 27 May 2025 13:35:21 -0700 Subject: [PATCH 13/23] add foreground command --- devservices/commands/foreground.py | 121 +++++++ devservices/main.py | 4 +- tests/commands/test_foreground.py | 560 +++++++++++++++++++++++++++++ 3 files changed, 684 insertions(+), 1 deletion(-) create mode 100644 devservices/commands/foreground.py create mode 100644 tests/commands/test_foreground.py diff --git a/devservices/commands/foreground.py b/devservices/commands/foreground.py new file mode 100644 index 00000000..78e98ed9 --- /dev/null +++ b/devservices/commands/foreground.py @@ -0,0 +1,121 @@ +from __future__ import annotations + +import os +import pty +import shlex +from argparse import _SubParsersAction +from argparse import ArgumentParser +from argparse import Namespace + +from sentry_sdk import capture_exception + +from devservices.constants import DependencyType +from devservices.constants import DEVSERVICES_DIR_NAME +from devservices.constants import PROGRAMS_CONF_FILE_NAME +from devservices.exceptions import ConfigError +from devservices.exceptions import ConfigNotFoundError +from devservices.exceptions import ServiceNotFoundError +from devservices.exceptions import SupervisorConfigError +from devservices.utils.console import Console +from devservices.utils.services import find_matching_service +from devservices.utils.state import State +from devservices.utils.state import StateTables +from devservices.utils.supervisor import SupervisorManager + + +def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: + parser = subparsers.add_parser("foreground", help="Run a service in the foreground") + parser.add_argument( + "program_name", help="Name of the program to run in the foreground" + ) + parser.set_defaults(func=foreground) + + +def foreground(args: Namespace) -> None: + """Run a service in the foreground.""" + console = Console() + program_name = args.program_name + try: + service = find_matching_service() + except ConfigNotFoundError as e: + capture_exception(e, level="info") + console.failure( + f"{str(e)}. Please specify a service (i.e. `devservices down sentry`) or run the command from a directory with a devservices configuration." + ) + exit(1) + except ConfigError as e: + capture_exception(e) + console.failure(str(e)) + exit(1) + except ServiceNotFoundError as e: + console.failure(str(e)) + exit(1) + modes = service.config.modes + state = State() + starting_services = set(state.get_service_entries(StateTables.STARTING_SERVICES)) + started_services = set(state.get_service_entries(StateTables.STARTED_SERVICES)) + active_services = starting_services.union(started_services) + if service.name not in active_services: + console.warning(f"{service.name} is not running") + return # + active_starting_modes = state.get_active_modes_for_service( + service.name, StateTables.STARTING_SERVICES + ) + active_started_modes = state.get_active_modes_for_service( + service.name, StateTables.STARTED_SERVICES + ) + active_modes = active_starting_modes or active_started_modes + mode_dependencies = set() + for active_mode in active_modes: + active_mode_dependencies = modes.get(active_mode, []) + mode_dependencies.update(active_mode_dependencies) + + supervisor_programs = [ + dep + for dep in mode_dependencies + if dep in service.config.dependencies + and service.config.dependencies[dep].dependency_type + == DependencyType.SUPERVISOR + ] + + if program_name not in supervisor_programs: + console.failure(f"Program {program_name} is not currently running.") + return + + programs_config_path = os.path.join( + service.repo_path, f"{DEVSERVICES_DIR_NAME}/{PROGRAMS_CONF_FILE_NAME}" + ) + + manager = SupervisorManager( + programs_config_path, + service_name=service.name, + ) + + try: + program_command = manager.get_program_command(program_name) + except SupervisorConfigError as e: + capture_exception(e, level="info") + console.failure(f"Error when getting program command: {str(e)}") + return + + try: + # Stop the supervisor process before running in foreground + console.info(f"Stopping {program_name} in supervisor") + manager.stop_process(program_name) + + console.info(f"Starting {program_name} in foreground") + argv = shlex.split(program_command) + + # Run the process in foreground + pty.spawn(argv) + + # Restart the process in background after foreground process exits + console.info(f"Restarting {program_name} in background") + manager.start_process(program_name) + + except Exception as e: + capture_exception(e) + console.failure(f"Error running {program_name} in foreground: {str(e)}") + # Ensure the process is restarted in case of failure + manager.start_process(program_name) + return diff --git a/devservices/main.py b/devservices/main.py index ad4d5026..deb6c310 100644 --- a/devservices/main.py +++ b/devservices/main.py @@ -20,6 +20,7 @@ from sentry_sdk.types import Hint from devservices.commands import down +from devservices.commands import foreground from devservices.commands import list_dependencies from devservices.commands import list_services from devservices.commands import logs @@ -75,7 +76,7 @@ def before_send_transaction(event: Event, hint: Hint) -> Event: if not disable_sentry: init( - dsn="https://56470da7302c16e83141f62f88e46449@o1.ingest.us.sentry.io/4507946704961536", + # dsn="https://56470da7302c16e83141f62f88e46449@o1.ingest.us.sentry.io/4507946704961536", traces_sample_rate=1.0, profiles_sample_rate=1.0, integrations=[ArgvIntegration()], @@ -150,6 +151,7 @@ def main() -> None: purge.add_parser(subparsers) serve.add_parser(subparsers) toggle.add_parser(subparsers) + foreground.add_parser(subparsers) reset.add_parser(subparsers) args = parser.parse_args() diff --git a/tests/commands/test_foreground.py b/tests/commands/test_foreground.py new file mode 100644 index 00000000..d95ae9c2 --- /dev/null +++ b/tests/commands/test_foreground.py @@ -0,0 +1,560 @@ +from __future__ import annotations + +import os +from argparse import Namespace +from pathlib import Path +from unittest import mock + +import pytest + +from devservices.commands.foreground import foreground +from devservices.constants import CONFIG_FILE_NAME +from devservices.constants import DEVSERVICES_DIR_NAME +from devservices.exceptions import ConfigError +from devservices.exceptions import ServiceNotFoundError +from devservices.exceptions import SupervisorConfigError +from devservices.utils.state import State +from devservices.utils.state import StateTables +from testing.utils import create_config_file +from testing.utils import create_programs_conf_file + + +@mock.patch("devservices.commands.foreground.pty.spawn") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_foreground_success( + mock_stop_process: mock.Mock, + mock_start_process: mock.Mock, + mock_pty_spawn: mock.Mock, + tmp_path: Path, +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "redis": {"description": "Redis"}, + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["redis", "worker"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command=python worker.py +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + args = Namespace(program_name="worker") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTED_SERVICES + ) + foreground(args) + + mock_stop_process.assert_called_once_with("worker") + mock_pty_spawn.assert_called_once_with(["python", "worker.py"]) + mock_start_process.assert_called_once_with("worker") + + +@mock.patch("devservices.commands.foreground.pty.spawn") +def test_foreground_service_not_running( + mock_pty_spawn: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["worker"]}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command=python worker.py +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + args = Namespace(program_name="worker") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + foreground(args) + + captured = capsys.readouterr() + assert "example-service is not running" in captured.out + + mock_pty_spawn.assert_not_called() + + +@mock.patch("devservices.commands.foreground.pty.spawn") +def test_foreground_program_not_in_supervisor_programs( + mock_pty_spawn: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "redis": {"description": "Redis"}, + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["redis", "worker"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command=python worker.py +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + args = Namespace(program_name="nonexistent") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTED_SERVICES + ) + foreground(args) + + captured = capsys.readouterr() + assert "Program nonexistent is not currently running." in captured.out + + mock_pty_spawn.assert_not_called() + + +@mock.patch("devservices.commands.foreground.pty.spawn") +def test_foreground_programs_conf_not_found( + mock_pty_spawn: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "redis": {"description": "Redis"}, + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["redis", "worker"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + args = Namespace(program_name="worker") + + with ( + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + pytest.raises(SystemExit), + ): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTED_SERVICES + ) + foreground(args) + + captured = capsys.readouterr() + assert ( + "\x1b[0;31mDependency 'worker' is not remote but is not defined in docker-compose services or programs file\x1b[0m\n" + == captured.out + ) + + mock_pty_spawn.assert_not_called() + + +def test_foreground_config_not_found_error( + capsys: pytest.CaptureFixture[str], + tmp_path: Path, +) -> None: + os.chdir(tmp_path) + + args = Namespace(program_name="worker") + + with pytest.raises(SystemExit): + foreground(args) + + captured = capsys.readouterr() + assert ( + f"\x1b[0;31mNo devservices configuration found in {tmp_path / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME}. Please specify a service (i.e. `devservices down sentry`) or run the command from a directory with a devservices configuration.\x1b[0m\n" + == captured.out + ) + + +@mock.patch("devservices.commands.foreground.find_matching_service") +def test_foreground_config_error( + mock_find_matching_service: mock.Mock, + capsys: pytest.CaptureFixture[str], +) -> None: + mock_find_matching_service.side_effect = ConfigError("Invalid config") + + args = Namespace(program_name="worker") + + with pytest.raises(SystemExit): + foreground(args) + + captured = capsys.readouterr() + assert "\x1b[0;31mInvalid config\x1b[0m\n" == captured.out + + +@mock.patch("devservices.commands.foreground.find_matching_service") +def test_foreground_service_not_found_error( + mock_find_matching_service: mock.Mock, + capsys: pytest.CaptureFixture[str], +) -> None: + mock_find_matching_service.side_effect = ServiceNotFoundError("Service not found") + + args = Namespace(program_name="worker") + + with pytest.raises(SystemExit): + foreground(args) + + captured = capsys.readouterr() + assert "\x1b[0;31mService not found\x1b[0m\n" == captured.out + + +@mock.patch("devservices.commands.foreground.pty.spawn") +@mock.patch("devservices.utils.supervisor.SupervisorManager.get_program_command") +def test_foreground_supervisor_config_error( + mock_get_program_command: mock.Mock, + mock_pty_spawn: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["worker"]}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command=python worker.py +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + mock_get_program_command.side_effect = SupervisorConfigError("Program config error") + + args = Namespace(program_name="worker") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTED_SERVICES + ) + foreground(args) + + # Verify output + captured = capsys.readouterr() + assert ( + "\x1b[0;31mError when getting program command: Program config error\x1b[0m\n" + == captured.out + ) + + # Verify pty.spawn was not called + mock_pty_spawn.assert_not_called() + + +@mock.patch("devservices.commands.foreground.pty.spawn") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_foreground_pty_spawn_exception( + mock_stop_process: mock.Mock, + mock_start_process: mock.Mock, + mock_pty_spawn: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["worker"]}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command=python worker.py +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + mock_pty_spawn.side_effect = Exception("Spawn failed") + + args = Namespace(program_name="worker") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTED_SERVICES + ) + foreground(args) + + captured = capsys.readouterr() + assert ( + "Stopping worker in supervisor\nStarting worker in foreground\n\x1b[0;31mError running worker in foreground: Spawn failed\x1b[0m\n" + == captured.out + ) + + mock_start_process.assert_called_once_with("worker") + mock_stop_process.assert_called_once_with("worker") + + +@mock.patch("devservices.commands.foreground.pty.spawn") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_foreground_with_starting_services( + mock_stop_process: mock.Mock, + mock_start_process: mock.Mock, + mock_pty_spawn: mock.Mock, + tmp_path: Path, +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["worker"]}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command=python worker.py +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + args = Namespace(program_name="worker") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTING_SERVICES + ) + foreground(args) + + # Verify calls + mock_pty_spawn.assert_called_once_with(["python", "worker.py"]) + + mock_start_process.assert_called_once_with("worker") + mock_stop_process.assert_called_once_with("worker") + + +@mock.patch("devservices.commands.foreground.pty.spawn") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_foreground_multiple_modes_and_dependencies( + mock_stop_process: mock.Mock, + mock_start_process: mock.Mock, + mock_pty_spawn: mock.Mock, + tmp_path: Path, +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "redis": {"description": "Redis"}, + "postgres": {"description": "Postgres"}, + "worker": {"description": "Worker"}, + "consumer": {"description": "Consumer"}, + }, + "modes": { + "default": ["redis", "worker"], + "full": ["redis", "postgres", "worker", "consumer"], + }, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + "postgres": {"image": "postgres:13"}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command=python worker.py +autostart=true +autorestart=true + +[program:consumer] +command=python consumer.py +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + args = Namespace(program_name="consumer") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + state = State() + state.update_service_entry( + "example-service", "full", StateTables.STARTED_SERVICES + ) + foreground(args) + + # Verify calls + mock_pty_spawn.assert_called_once_with(["python", "consumer.py"]) + + mock_start_process.assert_called_once_with("consumer") + mock_stop_process.assert_called_once_with("consumer") + + +@mock.patch("devservices.commands.foreground.pty.spawn") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +@mock.patch("devservices.utils.supervisor.SupervisorManager.stop_process") +def test_foreground_empty_command( + mock_stop_process: mock.Mock, + mock_start_process: mock.Mock, + mock_pty_spawn: mock.Mock, + tmp_path: Path, +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["worker"]}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command= +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + args = Namespace(program_name="worker") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + state = State() + state.update_service_entry( + "example-service", "default", StateTables.STARTED_SERVICES + ) + foreground(args) + + # Verify that shlex.split handles empty command correctly + mock_pty_spawn.assert_called_once_with([]) + + mock_start_process.assert_called_once_with("worker") + mock_stop_process.assert_called_once_with("worker") + + +@mock.patch("devservices.commands.foreground.pty.spawn") +def test_foreground_no_active_modes( + mock_pty_spawn: mock.Mock, + tmp_path: Path, +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "worker": {"description": "Worker"}, + }, + "modes": {"default": ["worker"], "other": []}, + }, + } + + service_path = tmp_path / "example-service" + create_config_file(service_path, config) + os.chdir(service_path) + + programs_config = """ +[program:worker] +command=python worker.py +autostart=true +autorestart=true +""" + create_programs_conf_file(service_path, programs_config) + + args = Namespace(program_name="worker") + + with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")): + state = State() + state.update_service_entry( + "example-service", "other", StateTables.STARTED_SERVICES + ) + foreground(args) + + # Should not call pty.spawn since no supervisor programs are active + mock_pty_spawn.assert_not_called() From 725af21375577e298e815fac8003941943c807ba Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 27 May 2025 13:36:14 -0700 Subject: [PATCH 14/23] uncomment dsn --- devservices/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devservices/main.py b/devservices/main.py index deb6c310..d59c96f3 100644 --- a/devservices/main.py +++ b/devservices/main.py @@ -76,7 +76,7 @@ def before_send_transaction(event: Event, hint: Hint) -> Event: if not disable_sentry: init( - # dsn="https://56470da7302c16e83141f62f88e46449@o1.ingest.us.sentry.io/4507946704961536", + dsn="https://56470da7302c16e83141f62f88e46449@o1.ingest.us.sentry.io/4507946704961536", traces_sample_rate=1.0, profiles_sample_rate=1.0, integrations=[ArgvIntegration()], From 4394c3c125daf4eff7e96b5ce272435511a6e773 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 27 May 2025 14:03:40 -0700 Subject: [PATCH 15/23] add case to not allow bringing up supervisor programs from outside the service repo directory --- devservices/commands/up.py | 5 ++++ tests/commands/test_up.py | 60 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/devservices/commands/up.py b/devservices/commands/up.py index f29d224e..46627f5d 100644 --- a/devservices/commands/up.py +++ b/devservices/commands/up.py @@ -311,6 +311,11 @@ def bring_up_supervisor_programs( ) -> None: if len(supervisor_programs) == 0: return + if not os.path.samefile(os.getcwd(), service.repo_path): + status.warning( + f"Cannot bring up supervisor programs from outside the service repository. Please run the command from the service repository ({service.repo_path})" + ) + return programs_config_path = os.path.join( service.repo_path, f"{DEVSERVICES_DIR_NAME}/{PROGRAMS_CONF_FILE_NAME}" ) diff --git a/tests/commands/test_up.py b/tests/commands/test_up.py index 939df892..36bdff32 100644 --- a/tests/commands/test_up.py +++ b/tests/commands/test_up.py @@ -2656,6 +2656,7 @@ def test_bring_up_supervisor_programs_no_programs_config( SupervisorConfigError, match=f"No programs.conf file found in {tmp_path / DEVSERVICES_DIR_NAME / PROGRAMS_CONF_FILE_NAME}", ): + os.chdir(tmp_path) bring_up_supervisor_programs(["supervisor-program"], service, status) mock_start_supervisor_daemon.assert_not_called() @@ -2733,6 +2734,7 @@ def test_bring_up_supervisor_programs_success( status = mock.MagicMock() + os.chdir(tmp_path) bring_up_supervisor_programs(["supervisor-program"], service, status) status.info.assert_has_calls( @@ -2745,3 +2747,61 @@ def test_bring_up_supervisor_programs_success( mock_start_supervisor_daemon.assert_called_once() mock_start_process.assert_called_once_with("supervisor-program") + + +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_supervisor_daemon") +@mock.patch("devservices.utils.supervisor.SupervisorManager.start_process") +def test_bring_up_supervisor_programs_wrong_directory( + mock_start_process: mock.Mock, + mock_start_supervisor_daemon: mock.Mock, + tmp_path: Path, +) -> None: + service_config = ServiceConfig( + version=0.1, + service_name="test-service", + dependencies={ + "supervisor-program": Dependency( + description="Supervisor program", + dependency_type=DependencyType.SUPERVISOR, + ), + }, + modes={"default": ["supervisor-program"]}, + ) + + service_repo_path = tmp_path / "service-repo" + service_repo_path.mkdir() + + service = Service( + name="test-service", + repo_path=str(service_repo_path), + config=service_config, + ) + + programs_conf_path = ( + service_repo_path / DEVSERVICES_DIR_NAME / PROGRAMS_CONF_FILE_NAME + ) + create_programs_conf_file( + programs_conf_path, + """ +[program:supervisor-program] +command=echo "Hello, world!" +""", + ) + + status = mock.MagicMock() + + different_dir = tmp_path / "different-dir" + different_dir.mkdir() + os.chdir(different_dir) + + # Call the function + bring_up_supervisor_programs(["supervisor-program"], service, status) + + status.warning.assert_called_once_with( + f"Cannot bring up supervisor programs from outside the service repository. Please run the command from the service repository ({service_repo_path})" + ) + status.info.assert_not_called() + status.failure.assert_not_called() + + mock_start_supervisor_daemon.assert_not_called() + mock_start_process.assert_not_called() From 25e2b1f06cfe43a6a719df72e5bee80e899aa225 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 27 May 2025 15:55:06 -0700 Subject: [PATCH 16/23] update status to support supervisor programs --- devservices/commands/status.py | 103 +++++++++- devservices/utils/supervisor.py | 64 ++++++ tests/commands/test_status.py | 333 +++++++++++++++++++++++++++++++- tests/utils/test_supervisor.py | 107 +++++++++- 4 files changed, 597 insertions(+), 10 deletions(-) diff --git a/devservices/commands/status.py b/devservices/commands/status.py index 59db155b..9da398d6 100644 --- a/devservices/commands/status.py +++ b/devservices/commands/status.py @@ -8,6 +8,7 @@ from argparse import ArgumentParser from argparse import Namespace from collections import namedtuple +from datetime import timedelta from typing import TypedDict from sentry_sdk import capture_exception @@ -19,6 +20,7 @@ from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY from devservices.constants import DEVSERVICES_DIR_NAME +from devservices.constants import PROGRAMS_CONF_FILE_NAME from devservices.exceptions import ConfigError from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import DependencyError @@ -37,6 +39,8 @@ from devservices.utils.state import ServiceRuntime from devservices.utils.state import State from devservices.utils.state import StateTables +from devservices.utils.supervisor import ProcessInfo +from devservices.utils.supervisor import SupervisorManager BASE_INDENTATION = " " @@ -99,8 +103,19 @@ def status(args: Namespace) -> None: console.warning(f"Status unavailable. {service.name} is not running standalone") return # Since exit(0) is captured as an internal_error by sentry + programs_config_path = os.path.join( + service.repo_path, f"{DEVSERVICES_DIR_NAME}/{PROGRAMS_CONF_FILE_NAME}" + ) + programs_status = [] + if os.path.exists(programs_config_path): + supervisor_manager = SupervisorManager( + programs_config_path, + service.name, + ) + programs_status = supervisor_manager.get_all_programs_status() + try: - status_tree = get_status_for_service(service) + status_tree = get_status_for_service(service, programs_status) except DependencyError as de: capture_exception(de) console.failure( @@ -114,7 +129,7 @@ def status(args: Namespace) -> None: console.info(status_tree) -def get_status_for_service(service: Service) -> str: +def get_status_for_service(service: Service, programs_status: list[ProcessInfo]) -> str: state = State() modes = service.config.modes @@ -141,7 +156,10 @@ def get_status_for_service(service: Service) -> str: docker_compose_service_to_status = parse_docker_compose_status(status_json_results) status_tree = generate_service_status_tree( - service.name, dependency_graph, docker_compose_service_to_status + service.name, + programs_status, + dependency_graph, + docker_compose_service_to_status, ) return status_tree @@ -188,6 +206,7 @@ def get_status_json_results( def generate_service_status_tree( service_name: str, + programs_status: list[ProcessInfo], dependency_graph: DependencyGraph, docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str = "", @@ -227,6 +246,7 @@ def generate_service_status_tree( output.append( process_service_with_containerized_runtime( dependency, + programs_status, docker_compose_service_to_status, indentation + BASE_INDENTATION, dependency_graph, @@ -261,6 +281,7 @@ def process_service_with_local_runtime( def process_service_with_containerized_runtime( dependency: DependencyNode, + programs_status: list[ProcessInfo], docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str, dependency_graph: DependencyGraph, @@ -268,13 +289,14 @@ def process_service_with_containerized_runtime( if len(dependency_graph.graph[dependency]) > 0: return generate_service_status_tree( dependency.name, + programs_status, dependency_graph, docker_compose_service_to_status, indentation, ) else: return generate_service_status_details( - dependency, docker_compose_service_to_status, indentation + dependency, programs_status, docker_compose_service_to_status, indentation ) @@ -301,16 +323,23 @@ def parse_docker_compose_status( def generate_service_status_details( dependency: DependencyNode, + programs_status: list[ProcessInfo], docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str, ) -> str: output = [f"{indentation}{Color.BOLD}{dependency.name}{Color.RESET}:"] + # Handle supervisor dependencies + if dependency.dependency_type == DependencyType.SUPERVISOR: + return generate_supervisor_status_details( + dependency, programs_status, indentation + ) + if dependency.name not in docker_compose_service_to_status: return "\n".join( [ *output, - f"{indentation}{BASE_INDENTATION}Type: container", + (f"{indentation}{BASE_INDENTATION}Type: container"), f"{indentation}{BASE_INDENTATION}Status: N/A", ] ) @@ -349,7 +378,7 @@ def handle_started_service(dependency: DependencyNode, indentation: str) -> str: f"{indentation}{BASE_INDENTATION}Runtime: local", ] ) - service_output = get_status_for_service(service_with_local_runtime) + service_output = get_status_for_service(service_with_local_runtime, []) return "\n".join( [f"{indentation}{line}" for line in service_output.splitlines()], ) @@ -365,3 +394,65 @@ def format_health(health: str) -> str: else Color.YELLOW ) return f"{color}{health}{Color.RESET}" + + +def generate_supervisor_status_details( + dependency: DependencyNode, + programs_status: list[ProcessInfo], + indentation: str, +) -> str: + """Generate status details for supervisor dependencies.""" + output = [f"{indentation}{Color.BOLD}{dependency.name}{Color.RESET}:"] + + # Find the specific program in the status list + program_status = None + for program in programs_status: + if program["name"] == dependency.name: + program_status = program + break + + if program_status is None: + return "\n".join( + [ + *output, + f"{indentation}{BASE_INDENTATION}Type: program", + f"{indentation}{BASE_INDENTATION}Status: N/A (program not found)", + ] + ) + + uptime_str = format_uptime(program_status["uptime"]) + + details = [ + "Type: program", + f"Status: {program_status['state_name'].lower()}", + f"PID: {program_status['pid'] if program_status['pid'] > 0 else 'N/A'}", + f"Uptime: {uptime_str}", + f"Group: {program_status['group'] or 'default'}", + ] + + if program_status["description"]: + details.append(f"Description: {program_status['description']}") + + output.extend(f"{indentation}{BASE_INDENTATION}{detail}" for detail in details) + + return "\n".join(output) + + +def format_uptime(uptime_seconds: int) -> str: + """Format uptime seconds into a human-readable string.""" + SECONDS_PER_MINUTE = 60 + SECONDS_PER_HOUR = 60 * SECONDS_PER_MINUTE + + td = timedelta(seconds=uptime_seconds) + days = td.days + hours, remainder = divmod(td.seconds, SECONDS_PER_HOUR) + minutes, seconds = divmod(remainder, SECONDS_PER_MINUTE) + + if days > 0: + return f"{days}d {hours}h {minutes}m {seconds}s" + elif hours > 0: + return f"{hours}h {minutes}m {seconds}s" + elif minutes > 0: + return f"{minutes}m {seconds}s" + else: + return f"{seconds}s" diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index 579929d8..11edb866 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -8,6 +8,7 @@ import time import xmlrpc.client from enum import IntEnum +from typing import TypedDict from supervisor.options import ServerOptions @@ -74,6 +75,20 @@ def make_connection( return UnixSocketHTTPConnection(self.socket_path) +class ProcessInfo(TypedDict): + """Status information for a supervisor process.""" + + name: str + state: int + state_name: str + description: str + pid: int + uptime: int + start_time: int + stop_time: int + group: str + + class SupervisorManager: def __init__(self, config_file_path: str, service_name: str) -> None: self.service_name = service_name @@ -262,3 +277,52 @@ def tail_program_logs(self, program_name: str) -> None: raise SupervisorError(f"Failed to tail logs for {program_name}: {str(e)}") except KeyboardInterrupt: pass + + def get_all_programs_status(self) -> list[ProcessInfo]: + """Get status information for all supervisor programs.""" + try: + client = self._get_rpc_client() + all_process_info = client.supervisor.getAllProcessInfo() + + # Validate that the response is a list before iterating for typechecking + if not isinstance(all_process_info, list): + return [] + + programs_status = [] + for process_info in all_process_info: + if not isinstance(process_info, dict): + continue + + # Extract basic fields with defaults + name = process_info.get("name", "") + state = process_info.get("state", SupervisorProcessState.UNKNOWN) + state_name = SupervisorProcessState(state).name + description = process_info.get("description", "") + pid = process_info.get("pid", 0) + group = process_info.get("group", "") + + # Calculate uptime for running processes + start_time = process_info.get("start", 0) + now = process_info.get("now", 0) + uptime = max(0, now - start_time) if start_time > 0 and now > 0 else 0 + + program_status: ProcessInfo = { + "name": name, + "state": state, + "state_name": state_name, + "description": description, + "pid": pid, + "uptime": uptime, + "start_time": start_time, + "stop_time": process_info.get("stop", 0), + "group": group, + } + programs_status.append(program_status) + + return programs_status + + except xmlrpc.client.Fault as e: + raise SupervisorError(f"Failed to get programs status: {e.faultString}") + except (SupervisorConnectionError, socket.error, ConnectionRefusedError): + # If supervisor is not running, return empty list + return [] diff --git a/tests/commands/test_status.py b/tests/commands/test_status.py index 52a577b3..1b47e6b7 100644 --- a/tests/commands/test_status.py +++ b/tests/commands/test_status.py @@ -8,8 +8,10 @@ import pytest +from devservices.commands.status import format_uptime from devservices.commands.status import generate_service_status_details from devservices.commands.status import generate_service_status_tree +from devservices.commands.status import generate_supervisor_status_details from devservices.commands.status import get_status_json_results from devservices.commands.status import handle_started_service from devservices.commands.status import parse_docker_compose_status @@ -30,8 +32,11 @@ from devservices.utils.services import Service from devservices.utils.state import State from devservices.utils.state import StateTables +from devservices.utils.supervisor import ProcessInfo +from devservices.utils.supervisor import SupervisorProcessState from testing.utils import create_config_file from testing.utils import create_mock_git_repo +from testing.utils import create_programs_conf_file from testing.utils import run_git_command @@ -179,8 +184,9 @@ def test_generate_service_status_details() -> None: ], } } + programs_status: list[ProcessInfo] = [] result = generate_service_status_details( - dependency, docker_compose_service_to_status, "" + dependency, programs_status, docker_compose_service_to_status, "" ) assert result == ( f"{Color.BOLD}test-service{Color.RESET}:\n" @@ -200,8 +206,9 @@ def test_generate_service_status_details_missing_status() -> None: dependency_type=DependencyType.SERVICE, ) docker_compose_service_to_status: dict[str, ServiceStatusOutput] = {} + programs_status: list[ProcessInfo] = [] result = generate_service_status_details( - dependency, docker_compose_service_to_status, "" + dependency, programs_status, docker_compose_service_to_status, "" ) assert result == ( f"{Color.BOLD}test-service{Color.RESET}:\n" @@ -249,6 +256,7 @@ def test_generate_service_status_tree_no_child_service( } result = generate_service_status_tree( "parent-service", + [], dependency_graph, docker_compose_service_to_status, "", @@ -333,6 +341,7 @@ def test_generate_service_status_tree_with_child_service( } result = generate_service_status_tree( "parent-service", + [], dependency_graph, docker_compose_service_to_status, "", @@ -448,6 +457,7 @@ def test_generate_service_status_tree_with_nested_child_services( } result = generate_service_status_tree( "grandparent-service", + [], dependency_graph, docker_compose_service_to_status, "", @@ -852,3 +862,322 @@ def test_status_service_not_running( captured = capsys.readouterr() assert "Status unavailable. test-service is not running standalone" in captured.out + + +def test_generate_supervisor_status_details_running_program() -> None: + dependency = DependencyNode( + name="test-program", + dependency_type=DependencyType.SUPERVISOR, + ) + programs_status: list[ProcessInfo] = [ + { + "name": "test-program", + "state": SupervisorProcessState.RUNNING, # RUNNING + "state_name": SupervisorProcessState.RUNNING.name, + "description": "Test program description", + "pid": 12345, + "uptime": 3661, # 1 hour, 1 minute, 1 second + "start_time": 1234567890, + "stop_time": 0, + "group": "test-group", + } + ] + + result = generate_supervisor_status_details(dependency, programs_status, "") + + assert result == ( + f"{Color.BOLD}test-program{Color.RESET}:\n" + " Type: program\n" + " Status: running\n" + " PID: 12345\n" + " Uptime: 1h 1m 1s\n" + " Group: test-group\n" + " Description: Test program description" + ) + + +def test_generate_supervisor_status_details_stopped_program() -> None: + """Test supervisor status details for a stopped program.""" + dependency = DependencyNode( + name="stopped-program", + dependency_type=DependencyType.SUPERVISOR, + ) + programs_status: list[ProcessInfo] = [ + { + "name": "stopped-program", + "state": SupervisorProcessState.STOPPED, # STOPPED + "state_name": SupervisorProcessState.STOPPED.name, + "description": "", + "pid": 0, + "uptime": 0, + "start_time": 0, + "stop_time": 1234567890, + "group": "", + } + ] + + result = generate_supervisor_status_details(dependency, programs_status, " ") + + assert result == ( + f" {Color.BOLD}stopped-program{Color.RESET}:\n" + " Type: program\n" + " Status: stopped\n" + " PID: N/A\n" + " Uptime: 0s\n" + " Group: default" + ) + + +def test_generate_supervisor_status_details_program_not_found() -> None: + """Test supervisor status details when program is not found.""" + dependency = DependencyNode( + name="missing-program", + dependency_type=DependencyType.SUPERVISOR, + ) + programs_status: list[ProcessInfo] = [ + { + "name": "other-program", + "state": SupervisorProcessState.RUNNING, + "state_name": SupervisorProcessState.RUNNING.name, + "description": "", + "pid": 12345, + "uptime": 100, + "start_time": 1234567890, + "stop_time": 0, + "group": "test", + } + ] + + result = generate_supervisor_status_details(dependency, programs_status, "") + + assert result == ( + f"{Color.BOLD}missing-program{Color.RESET}:\n" + " Type: program\n" + " Status: N/A (program not found)" + ) + + +def test_generate_supervisor_status_details_empty_programs_list() -> None: + """Test supervisor status details with empty programs list.""" + dependency = DependencyNode( + name="test-program", + dependency_type=DependencyType.SUPERVISOR, + ) + programs_status: list[ProcessInfo] = [] + + result = generate_supervisor_status_details(dependency, programs_status, "") + + assert result == ( + f"{Color.BOLD}test-program{Color.RESET}:\n" + " Type: program\n" + " Status: N/A (program not found)" + ) + + +def test_generate_service_status_details_supervisor_dependency() -> None: + """Test that supervisor dependencies are handled correctly in generate_service_status_details.""" + dependency = DependencyNode( + name="test-supervisor-program", + dependency_type=DependencyType.SUPERVISOR, + ) + programs_status: list[ProcessInfo] = [ + { + "name": "test-supervisor-program", + "state": SupervisorProcessState.RUNNING, + "state_name": SupervisorProcessState.RUNNING.name, + "description": "Test supervisor program", + "pid": 54321, + "uptime": 120, + "start_time": 1234567890, + "stop_time": 0, + "group": "supervisor-group", + } + ] + docker_compose_service_to_status: dict[str, ServiceStatusOutput] = {} + + result = generate_service_status_details( + dependency, programs_status, docker_compose_service_to_status, "" + ) + + assert result == ( + f"{Color.BOLD}test-supervisor-program{Color.RESET}:\n" + " Type: program\n" + " Status: running\n" + " PID: 54321\n" + " Uptime: 2m 0s\n" + " Group: supervisor-group\n" + " Description: Test supervisor program" + ) + + +def test_format_uptime_zero_seconds() -> None: + """Test format_uptime with zero seconds.""" + assert format_uptime(0) == "0s" + + +def test_format_uptime_seconds_only() -> None: + """Test format_uptime with seconds only.""" + assert format_uptime(30) == "30s" + + +def test_format_uptime_minutes_and_seconds() -> None: + """Test format_uptime with minutes and seconds.""" + assert format_uptime(90) == "1m 30s" # 1 minute 30 seconds + + +def test_format_uptime_hours_minutes_seconds() -> None: + """Test format_uptime with hours, minutes, and seconds.""" + assert format_uptime(3661) == "1h 1m 1s" # 1 hour 1 minute 1 second + + +def test_format_uptime_days_hours_minutes_seconds() -> None: + """Test format_uptime with days, hours, minutes, and seconds.""" + assert format_uptime(90061) == "1d 1h 1m 1s" # 1 day 1 hour 1 minute 1 second + + +def test_format_uptime_exact_hour() -> None: + """Test format_uptime with exact hour.""" + assert format_uptime(3600) == "1h 0m 0s" + + +def test_format_uptime_exact_day() -> None: + """Test format_uptime with exact day.""" + assert format_uptime(86400) == "1d 0h 0m 0s" + + +def test_format_uptime_large_values() -> None: + """Test format_uptime with large values.""" + # 10 days, 5 hours, 30 minutes, 45 seconds + uptime = 10 * 86400 + 5 * 3600 + 30 * 60 + 45 + assert format_uptime(uptime) == "10d 5h 30m 45s" + + +@mock.patch("devservices.commands.status.SupervisorManager") +@mock.patch("devservices.commands.status.get_status_json_results") +def test_status_with_supervisor_programs( + mock_get_status_json_results: mock.Mock, + mock_supervisor_manager: mock.Mock, + capsys: pytest.CaptureFixture[str], + tmp_path: Path, +) -> None: + with ( + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + mock.patch( + "devservices.utils.state.DEVSERVICES_LOCAL_DIR", str(tmp_path / "local") + ), + mock.patch( + "devservices.commands.status.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch( + "devservices.utils.services.get_coderoot", + return_value=str(tmp_path / "code"), + ), + ): + state = State() + state.update_service_entry( + "test-service", "default", StateTables.STARTED_SERVICES + ) + + test_service_repo_path = tmp_path / "code" / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "clickhouse": {"description": "Clickhouse"}, + "worker": {"description": "Background worker"}, + }, + "modes": {"default": ["clickhouse", "worker"]}, + }, + "services": { + "clickhouse": { + "image": "altinity/clickhouse-server:23.8.11.29.altinitystable" + }, + }, + } + create_config_file(test_service_repo_path, config) + + supervisor_program_config = """ +[program:worker] +command=python worker.py +autostart=false +autorestart=true +""" + + create_programs_conf_file(test_service_repo_path, supervisor_program_config) + + # Commit the config files so find_matching_service can discover them + run_git_command(["add", "."], cwd=test_service_repo_path) + run_git_command(["commit", "-m", "Add config"], cwd=test_service_repo_path) + + # Mock supervisor programs status + mock_programs_status: list[ProcessInfo] = [ + { + "name": "worker", + "state": SupervisorProcessState.RUNNING, + "state_name": "RUNNING", + "description": "Background worker process", + "pid": 12345, + "uptime": 3600, # 1 hour + "start_time": 1234567890, + "stop_time": 0, + "group": "workers", + }, + ] + + # Mock docker compose status for clickhouse (matching the config) + mock_docker_status = [ + subprocess.CompletedProcess( + args=[], + returncode=0, + stdout='{"Service": "clickhouse", "State": "running", "Health": "healthy", "Name": "test-service-clickhouse-1", "RunningFor": "2 hours ago", "Publishers": [{"URL": "127.0.0.1", "PublishedPort": 9000, "TargetPort": 9000, "Protocol": "tcp"}]}\n', + stderr="", + ) + ] + + # Set up mocks + mock_get_status_json_results.return_value = mock_docker_status + mock_supervisor_manager.return_value.get_all_programs_status.return_value = ( + mock_programs_status + ) + + # Change to service directory so find_matching_service can find the config + original_cwd = os.getcwd() + os.chdir(test_service_repo_path) + + try: + # Call the status function + args = Namespace(service_name="test-service") + status(args) + finally: + os.chdir(original_cwd) + + # Verify the output + captured = capsys.readouterr() + output = captured.out + + # Assert on the entire expected output block + expected_output = ( + f"{Color.BOLD}test-service{Color.RESET}:\n" + " Type: service\n" + " Runtime: local\n" + f" {Color.BOLD}clickhouse{Color.RESET}:\n" + " Type: container\n" + " Status: running\n" + f" Health: {Color.GREEN}healthy{Color.RESET}\n" + " Container: test-service-clickhouse-1\n" + " Uptime: 2 hours ago\n" + " Ports:\n" + " 127.0.0.1:9000 -> 9000/tcp\n" + f" {Color.BOLD}worker{Color.RESET}:\n" + " Type: program\n" + " Status: running\n" + " PID: 12345\n" + " Uptime: 1h 0m 0s\n" + " Group: workers\n" + " Description: Background worker process\n" + ) + assert output == expected_output diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index e0e3e055..4742a370 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -360,7 +360,7 @@ def test_get_program_command_program_not_found( @mock.patch("devservices.utils.supervisor.subprocess.run") @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def tail_program_logs_success( +def test_tail_program_logs_success( mock_rpc_client: mock.MagicMock, mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager, @@ -374,7 +374,8 @@ def tail_program_logs_success( "supervisorctl", "-c", supervisor_manager.config_file_path, - "fg", + "tail", + "-f", "test_program", ], check=True, @@ -508,3 +509,105 @@ def test_wait_for_supervisor_ready_timeout( assert mock_sleep.call_count == 5 assert mock_rpc_client.return_value.supervisor.getState.call_count == 5 + + +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_get_all_programs_status_success( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + """Test successful retrieval of all programs status.""" + mock_process_info = [ + { + "name": "program1", + "state": SupervisorProcessState.RUNNING, + "description": "Running program", + "pid": 1234, + "group": "default", + "start": 1000, + "now": 1100, + "stop": 0, + }, + { + "name": "program2", + "state": SupervisorProcessState.STOPPED, + "description": "Stopped program", + "pid": 0, + "group": "workers", + "start": 0, + "now": 1100, + "stop": 1050, + }, + ] + mock_rpc_client.return_value.supervisor.getAllProcessInfo.return_value = ( + mock_process_info + ) + + result = supervisor_manager.get_all_programs_status() + + assert len(result) == 2 + + expected_results = [ + { + "name": "program1", + "state": SupervisorProcessState.RUNNING, + "state_name": "RUNNING", + "description": "Running program", + "pid": 1234, + "group": "default", + "uptime": 100, # 1100 - 1000 + "start_time": 1000, + "stop_time": 0, + }, + { + "name": "program2", + "state": SupervisorProcessState.STOPPED, + "state_name": "STOPPED", + "description": "Stopped program", + "pid": 0, + "group": "workers", + "uptime": 0, # No uptime for stopped process + "start_time": 0, + "stop_time": 1050, + }, + ] + + for expected, actual in zip(expected_results, result): + assert actual == expected + + +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_get_all_programs_status_empty_list( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + """Test handling of empty programs list.""" + mock_rpc_client.return_value.supervisor.getAllProcessInfo.return_value = [] + + result = supervisor_manager.get_all_programs_status() + + assert result == [] + + +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_get_all_programs_status_xmlrpc_fault( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_rpc_client.return_value.supervisor.getAllProcessInfo.side_effect = ( + xmlrpc.client.Fault(1, "Test error") + ) + + with pytest.raises( + SupervisorError, match="Failed to get programs status: Test error" + ): + supervisor_manager.get_all_programs_status() + + +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_get_all_programs_status_connection_error( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_rpc_client.side_effect = SupervisorConnectionError("Connection failed") + + result = supervisor_manager.get_all_programs_status() + + # Should return empty list when supervisor is not running + assert result == [] From d6c46b518d1bd451fdd00c1957bf00136d3a0682 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 27 May 2025 16:00:51 -0700 Subject: [PATCH 17/23] update the convention --- devservices/commands/status.py | 56 +++++++++++++++++---------------- devservices/utils/supervisor.py | 8 ++--- tests/commands/test_status.py | 2 +- tests/utils/test_supervisor.py | 16 +++++----- 4 files changed, 42 insertions(+), 40 deletions(-) diff --git a/devservices/commands/status.py b/devservices/commands/status.py index 9da398d6..b0067c97 100644 --- a/devservices/commands/status.py +++ b/devservices/commands/status.py @@ -106,16 +106,16 @@ def status(args: Namespace) -> None: programs_config_path = os.path.join( service.repo_path, f"{DEVSERVICES_DIR_NAME}/{PROGRAMS_CONF_FILE_NAME}" ) - programs_status = [] + processes_status = [] if os.path.exists(programs_config_path): supervisor_manager = SupervisorManager( programs_config_path, service.name, ) - programs_status = supervisor_manager.get_all_programs_status() + processes_status = supervisor_manager.get_all_process_info() try: - status_tree = get_status_for_service(service, programs_status) + status_tree = get_status_for_service(service, processes_status) except DependencyError as de: capture_exception(de) console.failure( @@ -129,7 +129,9 @@ def status(args: Namespace) -> None: console.info(status_tree) -def get_status_for_service(service: Service, programs_status: list[ProcessInfo]) -> str: +def get_status_for_service( + service: Service, processes_status: list[ProcessInfo] +) -> str: state = State() modes = service.config.modes @@ -157,7 +159,7 @@ def get_status_for_service(service: Service, programs_status: list[ProcessInfo]) docker_compose_service_to_status = parse_docker_compose_status(status_json_results) status_tree = generate_service_status_tree( service.name, - programs_status, + processes_status, dependency_graph, docker_compose_service_to_status, ) @@ -206,7 +208,7 @@ def get_status_json_results( def generate_service_status_tree( service_name: str, - programs_status: list[ProcessInfo], + processes_status: list[ProcessInfo], dependency_graph: DependencyGraph, docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str = "", @@ -246,7 +248,7 @@ def generate_service_status_tree( output.append( process_service_with_containerized_runtime( dependency, - programs_status, + processes_status, docker_compose_service_to_status, indentation + BASE_INDENTATION, dependency_graph, @@ -281,7 +283,7 @@ def process_service_with_local_runtime( def process_service_with_containerized_runtime( dependency: DependencyNode, - programs_status: list[ProcessInfo], + processes_status: list[ProcessInfo], docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str, dependency_graph: DependencyGraph, @@ -289,14 +291,14 @@ def process_service_with_containerized_runtime( if len(dependency_graph.graph[dependency]) > 0: return generate_service_status_tree( dependency.name, - programs_status, + processes_status, dependency_graph, docker_compose_service_to_status, indentation, ) else: return generate_service_status_details( - dependency, programs_status, docker_compose_service_to_status, indentation + dependency, processes_status, docker_compose_service_to_status, indentation ) @@ -323,7 +325,7 @@ def parse_docker_compose_status( def generate_service_status_details( dependency: DependencyNode, - programs_status: list[ProcessInfo], + processes_status: list[ProcessInfo], docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str, ) -> str: @@ -332,7 +334,7 @@ def generate_service_status_details( # Handle supervisor dependencies if dependency.dependency_type == DependencyType.SUPERVISOR: return generate_supervisor_status_details( - dependency, programs_status, indentation + dependency, processes_status, indentation ) if dependency.name not in docker_compose_service_to_status: @@ -398,40 +400,40 @@ def format_health(health: str) -> str: def generate_supervisor_status_details( dependency: DependencyNode, - programs_status: list[ProcessInfo], + processes_status: list[ProcessInfo], indentation: str, ) -> str: """Generate status details for supervisor dependencies.""" output = [f"{indentation}{Color.BOLD}{dependency.name}{Color.RESET}:"] - # Find the specific program in the status list - program_status = None - for program in programs_status: - if program["name"] == dependency.name: - program_status = program + # Find the specific process in the status list + process_info = None + for process_status in processes_status: + if process_status["name"] == dependency.name: + process_info = process_status break - if program_status is None: + if process_info is None: return "\n".join( [ *output, - f"{indentation}{BASE_INDENTATION}Type: program", - f"{indentation}{BASE_INDENTATION}Status: N/A (program not found)", + f"{indentation}{BASE_INDENTATION}Type: process", + f"{indentation}{BASE_INDENTATION}Status: N/A (process not found)", ] ) - uptime_str = format_uptime(program_status["uptime"]) + uptime_str = format_uptime(process_info["uptime"]) details = [ "Type: program", - f"Status: {program_status['state_name'].lower()}", - f"PID: {program_status['pid'] if program_status['pid'] > 0 else 'N/A'}", + f"Status: {process_info['state_name'].lower()}", + f"PID: {process_info['pid'] if process_info['pid'] > 0 else 'N/A'}", f"Uptime: {uptime_str}", - f"Group: {program_status['group'] or 'default'}", + f"Group: {process_info['group'] or 'default'}", ] - if program_status["description"]: - details.append(f"Description: {program_status['description']}") + if process_info["description"]: + details.append(f"Description: {process_info['description']}") output.extend(f"{indentation}{BASE_INDENTATION}{detail}" for detail in details) diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index 11edb866..b5ae6a65 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -278,7 +278,7 @@ def tail_program_logs(self, program_name: str) -> None: except KeyboardInterrupt: pass - def get_all_programs_status(self) -> list[ProcessInfo]: + def get_all_process_info(self) -> list[ProcessInfo]: """Get status information for all supervisor programs.""" try: client = self._get_rpc_client() @@ -288,7 +288,7 @@ def get_all_programs_status(self) -> list[ProcessInfo]: if not isinstance(all_process_info, list): return [] - programs_status = [] + processes_status = [] for process_info in all_process_info: if not isinstance(process_info, dict): continue @@ -317,9 +317,9 @@ def get_all_programs_status(self) -> list[ProcessInfo]: "stop_time": process_info.get("stop", 0), "group": group, } - programs_status.append(program_status) + processes_status.append(program_status) - return programs_status + return processes_status except xmlrpc.client.Fault as e: raise SupervisorError(f"Failed to get programs status: {e.faultString}") diff --git a/tests/commands/test_status.py b/tests/commands/test_status.py index 1b47e6b7..f72a4bc6 100644 --- a/tests/commands/test_status.py +++ b/tests/commands/test_status.py @@ -1140,7 +1140,7 @@ def test_status_with_supervisor_programs( # Set up mocks mock_get_status_json_results.return_value = mock_docker_status - mock_supervisor_manager.return_value.get_all_programs_status.return_value = ( + mock_supervisor_manager.return_value.get_all_process_info.return_value = ( mock_programs_status ) diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index 4742a370..1d9a491b 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -512,7 +512,7 @@ def test_wait_for_supervisor_ready_timeout( @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_get_all_programs_status_success( +def test_get_all_process_info_success( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: """Test successful retrieval of all programs status.""" @@ -542,7 +542,7 @@ def test_get_all_programs_status_success( mock_process_info ) - result = supervisor_manager.get_all_programs_status() + result = supervisor_manager.get_all_process_info() assert len(result) == 2 @@ -576,19 +576,19 @@ def test_get_all_programs_status_success( @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_get_all_programs_status_empty_list( +def test_get_all_process_info_empty_list( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: """Test handling of empty programs list.""" mock_rpc_client.return_value.supervisor.getAllProcessInfo.return_value = [] - result = supervisor_manager.get_all_programs_status() + result = supervisor_manager.get_all_process_info() assert result == [] @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_get_all_programs_status_xmlrpc_fault( +def test_get_all_process_info_xmlrpc_fault( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: mock_rpc_client.return_value.supervisor.getAllProcessInfo.side_effect = ( @@ -598,16 +598,16 @@ def test_get_all_programs_status_xmlrpc_fault( with pytest.raises( SupervisorError, match="Failed to get programs status: Test error" ): - supervisor_manager.get_all_programs_status() + supervisor_manager.get_all_process_info() @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_get_all_programs_status_connection_error( +def test_get_all_process_info_connection_error( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: mock_rpc_client.side_effect = SupervisorConnectionError("Connection failed") - result = supervisor_manager.get_all_programs_status() + result = supervisor_manager.get_all_process_info() # Should return empty list when supervisor is not running assert result == [] From e3f5c849d66c8dd138a7b620c0ad5cba80d3140f Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 27 May 2025 16:04:36 -0700 Subject: [PATCH 18/23] update tests --- tests/commands/test_status.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/commands/test_status.py b/tests/commands/test_status.py index f72a4bc6..c70bbeac 100644 --- a/tests/commands/test_status.py +++ b/tests/commands/test_status.py @@ -952,8 +952,8 @@ def test_generate_supervisor_status_details_program_not_found() -> None: assert result == ( f"{Color.BOLD}missing-program{Color.RESET}:\n" - " Type: program\n" - " Status: N/A (program not found)" + " Type: process\n" + " Status: N/A (process not found)" ) @@ -969,8 +969,8 @@ def test_generate_supervisor_status_details_empty_programs_list() -> None: assert result == ( f"{Color.BOLD}test-program{Color.RESET}:\n" - " Type: program\n" - " Status: N/A (program not found)" + " Type: process\n" + " Status: N/A (process not found)" ) From e9df9893a233fb2e54f25c99ecbe3de871d388be Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Wed, 28 May 2025 16:01:08 -0700 Subject: [PATCH 19/23] update logs to support supervisor programs --- devservices/commands/logs.py | 74 +++- devservices/utils/supervisor.py | 32 +- tests/commands/test_logs.py | 678 +++++++++++++++++++++++++++----- tests/utils/test_supervisor.py | 107 +++++ 4 files changed, 794 insertions(+), 97 deletions(-) diff --git a/devservices/commands/logs.py b/devservices/commands/logs.py index 9357595e..3bb7df1b 100644 --- a/devservices/commands/logs.py +++ b/devservices/commands/logs.py @@ -11,15 +11,18 @@ from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION +from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY from devservices.constants import DEVSERVICES_DIR_NAME from devservices.constants import MAX_LOG_LINES +from devservices.constants import PROGRAMS_CONF_FILE_NAME from devservices.exceptions import ConfigError from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import DependencyError from devservices.exceptions import DockerComposeError from devservices.exceptions import ServiceNotFoundError +from devservices.exceptions import SupervisorError from devservices.utils.console import Console from devservices.utils.dependencies import install_and_verify_dependencies from devservices.utils.dependencies import InstalledRemoteDependency @@ -29,6 +32,7 @@ from devservices.utils.services import Service from devservices.utils.state import State from devservices.utils.state import StateTables +from devservices.utils.supervisor import SupervisorManager def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: @@ -61,13 +65,25 @@ def logs(args: Namespace) -> None: except ServiceNotFoundError as e: console.failure(str(e)) exit(1) + state = State() modes = service.config.modes - # TODO: allow custom modes to be used - mode_to_use = "default" - mode_dependencies = modes[mode_to_use] + starting_modes = set( + state.get_active_modes_for_service(service.name, StateTables.STARTING_SERVICES) + ) + started_modes = set( + state.get_active_modes_for_service(service.name, StateTables.STARTED_SERVICES) + ) + active_modes = starting_modes.union(started_modes) + mode_dependencies = set() + for active_mode in active_modes: + active_mode_dependencies = modes.get(active_mode, []) + mode_dependencies.update(active_mode_dependencies) + + # If no active modes found but service is running, fall back to default mode + if not mode_dependencies and "default" in modes: + mode_dependencies.update(modes["default"]) - state = State() starting_services = set(state.get_service_entries(StateTables.STARTING_SERVICES)) started_services = set(state.get_service_entries(StateTables.STARTED_SERVICES)) running_services = starting_services.union(started_services) @@ -76,7 +92,9 @@ def logs(args: Namespace) -> None: return try: - remote_dependencies = install_and_verify_dependencies(service) + remote_dependencies = install_and_verify_dependencies( + service, modes=list(active_modes) + ) except DependencyError as de: capture_exception(de) console.failure( @@ -84,7 +102,7 @@ def logs(args: Namespace) -> None: ) exit(1) try: - logs_output = _logs(service, remote_dependencies, mode_dependencies) + logs_output = _logs(service, remote_dependencies, list(mode_dependencies)) except DockerComposeError as dce: capture_exception(dce, level="info") console.failure(f"Failed to get logs for {service.name}: {dce.stderr}") @@ -94,6 +112,22 @@ def logs(args: Namespace) -> None: if log_stdout is not None: console.info(log_stdout) + # Get supervisor program logs + supervisor_programs = [ + dep + for dep in mode_dependencies + if dep in service.config.dependencies + and service.config.dependencies[dep].dependency_type + == DependencyType.SUPERVISOR + ] + + if supervisor_programs: + supervisor_logs = _supervisor_logs(service, supervisor_programs) + for program_name, log_content in supervisor_logs.items(): + if log_content: + console.info(f"=== Logs for supervisor program: {program_name} ===") + console.info(log_content) + def _logs( service: Service, @@ -133,3 +167,31 @@ def _logs( cmd_outputs.append(future.result()) return cmd_outputs + + +def _supervisor_logs( + service: Service, supervisor_programs: list[str] +) -> dict[str, str]: + if not supervisor_programs: + return {} + + supervisor_logs = {} + + programs_config_path = os.path.join( + service.repo_path, DEVSERVICES_DIR_NAME, PROGRAMS_CONF_FILE_NAME + ) + + manager = SupervisorManager(programs_config_path, service_name=service.name) + + for program_name in supervisor_programs: + try: + log_content = manager.get_program_logs(program_name) + supervisor_logs[program_name] = log_content + except SupervisorError as e: + # Log the error but continue with other programs + capture_exception(e) + supervisor_logs[ + program_name + ] = f"Error getting logs for {program_name}: {str(e)}" + + return supervisor_logs diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index b5ae6a65..a16e305d 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -254,14 +254,39 @@ def get_program_command(self, program_name: str) -> str: return proc.command raise SupervisorConfigError(f"Program {program_name} not found in config") + def get_program_logs(self, program_name: str) -> str: + """Get logs for a supervisor program as text output.""" + if not self._is_program_running(program_name): + return f"Program {program_name} is not running" + + try: + # Use supervisorctl tail command to get logs + result = subprocess.run( + [ + "supervisorctl", + "-c", + self.config_file_path, + "tail", + program_name, + ], + capture_output=True, + text=True, + check=True, + ) + return result.stdout + except subprocess.CalledProcessError as e: + raise SupervisorError(f"Failed to get logs for {program_name}: {str(e)}") + def tail_program_logs(self, program_name: str) -> None: + """Tail logs for a supervisor program in real-time (follow mode).""" + console = Console() + if not self._is_program_running(program_name): - console = Console() - console.failure(f"Program {program_name} is not running") + console.info(f"Program {program_name} is not running") return try: - # Use supervisorctl tail command + # Use supervisorctl tail -f command to follow logs subprocess.run( [ "supervisorctl", @@ -276,6 +301,7 @@ def tail_program_logs(self, program_name: str) -> None: except subprocess.CalledProcessError as e: raise SupervisorError(f"Failed to tail logs for {program_name}: {str(e)}") except KeyboardInterrupt: + # Handle Ctrl+C gracefully when following logs pass def get_all_process_info(self) -> list[ProcessInfo]: diff --git a/tests/commands/test_logs.py b/tests/commands/test_logs.py index 9eda8ad5..ac437f7e 100644 --- a/tests/commands/test_logs.py +++ b/tests/commands/test_logs.py @@ -8,58 +8,71 @@ import pytest +from devservices.commands.logs import _supervisor_logs from devservices.commands.logs import logs -from devservices.configs.service_config import Dependency -from devservices.configs.service_config import ServiceConfig +from devservices.configs.service_config import load_service_config_from_file from devservices.constants import CONFIG_FILE_NAME -from devservices.constants import DependencyType from devservices.constants import DEVSERVICES_DIR_NAME -from devservices.exceptions import ConfigError -from devservices.exceptions import ServiceNotFoundError +from devservices.constants import PROGRAMS_CONF_FILE_NAME +from devservices.exceptions import DependencyError +from devservices.exceptions import DockerComposeError +from devservices.exceptions import SupervisorError from devservices.utils.services import Service from devservices.utils.state import StateTables +from testing.utils import create_config_file +from testing.utils import create_mock_git_repo +from testing.utils import create_programs_conf_file @mock.patch("devservices.commands.logs.get_docker_compose_commands_to_run") -@mock.patch("devservices.commands.logs.find_matching_service") @mock.patch("devservices.utils.state.State.get_service_entries") @mock.patch("devservices.commands.logs.install_and_verify_dependencies") def test_logs_no_specified_service_not_running( mock_install_and_verify_dependencies: mock.Mock, mock_get_service_entries: mock.Mock, - mock_find_matching_service: mock.Mock, mock_get_docker_compose_commands_to_run: mock.Mock, capsys: pytest.CaptureFixture[str], tmp_path: Path, ) -> None: - with mock.patch( - "devservices.commands.logs.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), + with ( + mock.patch( + "devservices.commands.logs.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch( + "devservices.utils.services.get_coderoot", + str(tmp_path / "code"), + ), ): - args = Namespace(service_name=None) - mock_service = Service( - name="example-service", - config=ServiceConfig( - version=0.1, - service_name="example-service", - dependencies={ - "redis": Dependency( - description="Redis", dependency_type=DependencyType.COMPOSE - ), - "clickhouse": Dependency( - description="Clickhouse", dependency_type=DependencyType.COMPOSE - ), + # Create a test service + test_service_repo_path = tmp_path / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "redis": {"description": "Redis"}, + "clickhouse": {"description": "Clickhouse"}, }, - modes={"default": ["redis", "clickhouse"]}, - ), - repo_path=str(tmp_path / "example-service"), - ) + "modes": {"default": ["redis", "clickhouse"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + "clickhouse": { + "image": "altinity/clickhouse-server:23.8.11.29.altinitystable" + }, + }, + } + create_config_file(test_service_repo_path, config) + + # Change to the service directory and run logs + os.chdir(test_service_repo_path) + args = Namespace(service_name=None) mock_get_service_entries.return_value = [] - mock_find_matching_service.return_value = mock_service logs(args) - mock_find_matching_service.assert_called_once_with(None) mock_get_service_entries.assert_has_calls( [ mock.call( @@ -74,12 +87,10 @@ def test_logs_no_specified_service_not_running( mock_get_docker_compose_commands_to_run.assert_not_called() captured = capsys.readouterr() - assert "Service example-service is not running" in captured.out + assert "\x1b[0;33mService test-service is not running\x1b[0m\n" == captured.out -# TODO: Ideally we should also have tests that don't mock the intermediate functions @mock.patch("devservices.commands.logs.run_cmd") -@mock.patch("devservices.commands.logs.find_matching_service") @mock.patch("devservices.utils.state.State.get_service_entries") @mock.patch("devservices.commands.logs.install_and_verify_dependencies") @mock.patch("devservices.utils.docker_compose.get_non_remote_services") @@ -87,50 +98,56 @@ def test_logs_no_specified_service_success( mock_get_non_remote_services: mock.Mock, mock_install_and_verify_dependencies: mock.Mock, mock_get_service_entries: mock.Mock, - mock_find_matching_service: mock.Mock, mock_run_cmd: mock.Mock, capsys: pytest.CaptureFixture[str], tmp_path: Path, ) -> None: - with mock.patch( - "devservices.commands.logs.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), + with ( + mock.patch( + "devservices.commands.logs.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch( + "devservices.utils.services.get_coderoot", + str(tmp_path / "code"), + ), ): - args = Namespace(service_name=None) - mock_service = Service( - name="example-service", - config=ServiceConfig( - version=0.1, - service_name="example-service", - dependencies={ - "redis": Dependency( - description="Redis", dependency_type=DependencyType.COMPOSE - ), - "clickhouse": Dependency( - description="Clickhouse", dependency_type=DependencyType.COMPOSE - ), + # Create a test service + test_service_repo_path = tmp_path / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "redis": {"description": "Redis"}, + "clickhouse": {"description": "Clickhouse"}, }, - modes={"default": ["redis", "clickhouse"]}, - ), - repo_path=str(tmp_path / "example-service"), - ) - mock_install_and_verify_dependencies.return_value = {} - mock_get_service_entries.return_value = ["example-service"] - mock_find_matching_service.return_value = mock_service + "modes": {"default": ["redis", "clickhouse"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + "clickhouse": { + "image": "altinity/clickhouse-server:23.8.11.29.altinitystable" + }, + }, + } + create_config_file(test_service_repo_path, config) + + # Change to the service directory and run logs + os.chdir(test_service_repo_path) + args = Namespace(service_name=None) + mock_install_and_verify_dependencies.return_value = set() + mock_get_service_entries.return_value = ["test-service"] mock_get_non_remote_services.return_value = {"redis", "clickhouse"} mock_run_cmd.return_value = subprocess.CompletedProcess( args=[ "docker", "compose", "-p", - "example-service", + "test-service", "-f", - str( - tmp_path - / "example-service" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ), + str(test_service_repo_path / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME), "logs", "clickhouse", "redis", @@ -143,7 +160,6 @@ def test_logs_no_specified_service_success( logs(args) - mock_find_matching_service.assert_called_once_with(None) mock_get_service_entries.assert_has_calls( [ mock.call( @@ -160,14 +176,9 @@ def test_logs_no_specified_service_success( "docker", "compose", "-p", - "example-service", + "test-service", "-f", - str( - tmp_path - / "example-service" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ), + str(test_service_repo_path / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME), "logs", "clickhouse", "redis", @@ -201,32 +212,523 @@ def test_logs_no_config_file( ) -@mock.patch("devservices.commands.logs.find_matching_service") def test_logs_config_error( - find_matching_service_mock: mock.Mock, capsys: pytest.CaptureFixture[str], + tmp_path: Path, ) -> None: - find_matching_service_mock.side_effect = ConfigError("Config error") - args = Namespace(service_name="example-service") + with mock.patch( + "devservices.utils.services.get_coderoot", + return_value=str(tmp_path / "code"), + ): + # Create an invalid config file + test_service_repo_path = tmp_path / "code" / "test-service" + create_mock_git_repo("invalid_repo", test_service_repo_path) - with pytest.raises(SystemExit): - logs(args) + args = Namespace(service_name="test-service") - find_matching_service_mock.assert_called_once_with("example-service") - captured = capsys.readouterr() - assert "Config error" in captured.out.strip() + with pytest.raises(SystemExit): + logs(args) + + captured = capsys.readouterr() + assert "test-service was found with an invalid config" in captured.out -@mock.patch("devservices.commands.logs.find_matching_service") def test_logs_service_not_found_error( - find_matching_service_mock: mock.Mock, capsys: pytest.CaptureFixture[str] + capsys: pytest.CaptureFixture[str], tmp_path: Path ) -> None: - find_matching_service_mock.side_effect = ServiceNotFoundError("Service not found") - args = Namespace(service_name="example-service") + with mock.patch( + "devservices.utils.services.get_coderoot", + return_value=str(tmp_path / "code"), + ): + # Create empty code directory + os.makedirs(tmp_path / "code", exist_ok=True) + + args = Namespace(service_name="nonexistent-service") + + with pytest.raises(SystemExit): + logs(args) + + captured = capsys.readouterr() + assert ( + "\x1b[0;31mService 'nonexistent-service' not found.\x1b[0m" + == captured.out.strip() + ) + + +@mock.patch("devservices.utils.state.State.get_service_entries") +@mock.patch("devservices.commands.logs.install_and_verify_dependencies") +def test_logs_dependency_error( + mock_install_and_verify_dependencies: mock.Mock, + mock_get_service_entries: mock.Mock, + capsys: pytest.CaptureFixture[str], + tmp_path: Path, +) -> None: + with ( + mock.patch( + "devservices.commands.logs.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch( + "devservices.utils.services.get_coderoot", + return_value=str(tmp_path / "code"), + ), + ): + # Create a test service + test_service_repo_path = tmp_path / "code" / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "redis": {"description": "Redis"}, + }, + "modes": {"default": ["redis"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + }, + } + create_config_file(test_service_repo_path, config) + + args = Namespace(service_name="test-service") + mock_get_service_entries.return_value = ["test-service"] + mock_install_and_verify_dependencies.side_effect = DependencyError( + repo_name="test-service", + repo_link=str(tmp_path), + branch="main", + stderr="Dependency installation failed", + ) + + with pytest.raises(SystemExit): + logs(args) + + captured = capsys.readouterr() + assert ( + f"\x1b[0;31mDependencyError: test-service ({tmp_path}) on main. If this error persists, try running `devservices purge`\x1b[0m\n" + == captured.out + ) + + +@mock.patch("devservices.commands.logs._logs") +@mock.patch("devservices.utils.state.State.get_service_entries") +@mock.patch("devservices.commands.logs.install_and_verify_dependencies") +def test_logs_docker_compose_error( + mock_install_and_verify_dependencies: mock.Mock, + mock_get_service_entries: mock.Mock, + mock_logs: mock.Mock, + capsys: pytest.CaptureFixture[str], + tmp_path: Path, +) -> None: + with ( + mock.patch( + "devservices.commands.logs.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch( + "devservices.utils.services.get_coderoot", + return_value=str(tmp_path / "code"), + ), + ): + # Create a test service + test_service_repo_path = tmp_path / "code" / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "redis": {"description": "Redis"}, + }, + "modes": {"default": ["redis"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + }, + } + create_config_file(test_service_repo_path, config) + + args = Namespace(service_name="test-service") + mock_get_service_entries.return_value = ["test-service"] + mock_install_and_verify_dependencies.return_value = set() + mock_logs.side_effect = DockerComposeError( + command="docker compose logs", + returncode=1, + stdout="", + stderr="stderr_output", + ) + + with pytest.raises(SystemExit): + logs(args) + + captured = capsys.readouterr() + assert ( + "\x1b[0;31mFailed to get logs for test-service: stderr_output\x1b[0m\n" + == captured.out + ) + + +@mock.patch("devservices.commands.logs._supervisor_logs") +@mock.patch("devservices.commands.logs._logs") +@mock.patch("devservices.utils.state.State.get_service_entries") +@mock.patch("devservices.commands.logs.install_and_verify_dependencies") +def test_logs_with_supervisor_dependencies( + mock_install_and_verify_dependencies: mock.Mock, + mock_get_service_entries: mock.Mock, + mock_logs: mock.Mock, + mock_supervisor_logs: mock.Mock, + capsys: pytest.CaptureFixture[str], + tmp_path: Path, +) -> None: + with ( + mock.patch( + "devservices.commands.logs.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch( + "devservices.utils.services.get_coderoot", + return_value=str(tmp_path / "code"), + ), + ): + # Create a test service with supervisor dependencies + test_service_repo_path = tmp_path / "code" / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "redis": {"description": "Redis", "dependency_type": "compose"}, + "worker": { + "description": "Worker", + "dependency_type": "supervisor", + }, + }, + "modes": {"default": ["redis", "worker"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + }, + } + create_config_file(test_service_repo_path, config) + + supervisor_config = """ +[program:worker] +command=python run worker +""" + + create_programs_conf_file(test_service_repo_path, supervisor_config) + + args = Namespace(service_name="test-service") + mock_get_service_entries.return_value = ["test-service"] + mock_install_and_verify_dependencies.return_value = set() + mock_logs.return_value = [ + subprocess.CompletedProcess( + args=["docker", "compose", "logs"], + returncode=0, + stdout="docker logs output", + ) + ] + mock_supervisor_logs.return_value = {"worker": "supervisor worker logs output"} - with pytest.raises(SystemExit): logs(args) - find_matching_service_mock.assert_called_once_with("example-service") - captured = capsys.readouterr() - assert "Service not found" in captured.out.strip() + # Verify that supervisor logs were called with the right service + mock_supervisor_logs.assert_called_once() + supervisor_call_args = mock_supervisor_logs.call_args[0] + assert supervisor_call_args[0].name == "test-service" + assert supervisor_call_args[1] == ["worker"] + + captured = capsys.readouterr() + assert ( + "docker logs output\n=== Logs for supervisor program: worker ===\nsupervisor worker logs output\n" + == captured.out + ) + + +@mock.patch("devservices.commands.logs.SupervisorManager") +def test_supervisor_logs_no_config_file( + mock_supervisor_manager_class: mock.Mock, + tmp_path: Path, +) -> None: + test_service_repo_path = tmp_path / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "worker": {"description": "Worker", "dependency_type": "supervisor"}, + }, + "modes": {"default": ["worker"]}, + }, + } + create_config_file(test_service_repo_path, config) + + supervisor_config = """ +[program:worker] +command=python run worker +""" + + # Create programs.conf file for supervisor dependency + create_programs_conf_file(test_service_repo_path, supervisor_config) + + service_config = load_service_config_from_file(str(test_service_repo_path)) + service = Service( + name=service_config.service_name, + repo_path=str(test_service_repo_path), + config=service_config, + ) + + # Mock SupervisorManager to raise an exception when config file doesn't exist + mock_supervisor_manager_class.side_effect = Exception("Config file not found") + + # The current implementation doesn't handle general exceptions during manager creation + # so this will raise an exception + with pytest.raises(Exception, match="Config file not found"): + _supervisor_logs(service, ["worker"]) + + +@mock.patch("devservices.commands.logs.SupervisorManager") +def test_supervisor_logs_manager_creation_error( + mock_supervisor_manager_class: mock.Mock, + tmp_path: Path, +) -> None: + # Create a test service + test_service_repo_path = tmp_path / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "worker": {"description": "Worker", "dependency_type": "supervisor"}, + }, + "modes": {"default": ["worker"]}, + }, + } + create_config_file(test_service_repo_path, config) + + supervisor_config = """ +[program:worker] +command=python run worker +""" + + # Create programs.conf file for supervisor dependency + create_programs_conf_file(test_service_repo_path, supervisor_config) + + # Load the service from config + from devservices.configs.service_config import load_service_config_from_file + + service_config = load_service_config_from_file(str(test_service_repo_path)) + service = Service( + name=service_config.service_name, + repo_path=str(test_service_repo_path), + config=service_config, + ) + + # Mock SupervisorManager to raise an exception during creation + mock_supervisor_manager_class.side_effect = Exception("General supervisor error") + + # The current implementation doesn't handle general exceptions during manager creation + # so this will raise an exception + with pytest.raises(Exception, match="General supervisor error"): + _supervisor_logs(service, ["worker"]) + + +def test_supervisor_logs_empty_programs_list(tmp_path: Path) -> None: + # Create a test service + test_service_repo_path = tmp_path / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": {}, + "modes": {"default": []}, + }, + } + create_config_file(test_service_repo_path, config) + + # Load the service from config + from devservices.configs.service_config import load_service_config_from_file + + service_config = load_service_config_from_file(str(test_service_repo_path)) + service = Service( + name=service_config.service_name, + repo_path=str(test_service_repo_path), + config=service_config, + ) + + result = _supervisor_logs(service, []) + + assert result == {} + + +@mock.patch("devservices.utils.state.State.get_active_modes_for_service") +@mock.patch("devservices.utils.state.State.get_service_entries") +@mock.patch("devservices.commands.logs.install_and_verify_dependencies") +@mock.patch("devservices.commands.logs._logs") +def test_logs_with_active_modes( + mock_logs: mock.Mock, + mock_install_and_verify_dependencies: mock.Mock, + mock_get_service_entries: mock.Mock, + mock_get_active_modes: mock.Mock, + tmp_path: Path, +) -> None: + with ( + mock.patch( + "devservices.commands.logs.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ), + mock.patch( + "devservices.utils.services.get_coderoot", + return_value=str(tmp_path / "code"), + ), + ): + # Create a test service with multiple modes + test_service_repo_path = tmp_path / "code" / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "redis": {"description": "Redis"}, + "postgres": {"description": "Postgres"}, + }, + "modes": { + "default": ["redis"], + "full": ["redis", "postgres"], + }, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + "postgres": {"image": "postgres:13"}, + }, + } + create_config_file(test_service_repo_path, config) + + args = Namespace(service_name="test-service") + mock_get_service_entries.return_value = ["test-service"] + mock_get_active_modes.side_effect = [ + ["full"], # starting modes + [], # started modes + ] + mock_install_and_verify_dependencies.return_value = set() + mock_logs.return_value = [] + + logs(args) + + mock_install_and_verify_dependencies.assert_called_once() + call_args = mock_install_and_verify_dependencies.call_args[0] + assert call_args[0].name == "test-service" + assert call_args[0].config.modes["full"] == ["redis", "postgres"] + + +@mock.patch("devservices.commands.logs.SupervisorManager") +def test_supervisor_logs_success( + mock_supervisor_manager_class: mock.Mock, + tmp_path: Path, +) -> None: + test_service_repo_path = tmp_path / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "worker": {"description": "Worker", "dependency_type": "supervisor"}, + }, + "modes": {"default": ["worker"]}, + }, + } + create_config_file(test_service_repo_path, config) + + # Create the programs.conf file + programs_config_path = ( + test_service_repo_path / DEVSERVICES_DIR_NAME / PROGRAMS_CONF_FILE_NAME + ) + supervisor_config = """ +[program:worker] +command=python run worker +""" + + create_programs_conf_file(test_service_repo_path, supervisor_config) + + # Load the service from config + from devservices.configs.service_config import load_service_config_from_file + + service_config = load_service_config_from_file(str(test_service_repo_path)) + service = Service( + name=service_config.service_name, + repo_path=str(test_service_repo_path), + config=service_config, + ) + + mock_manager = mock.Mock() + mock_manager.get_program_logs.return_value = "worker program logs" + mock_supervisor_manager_class.return_value = mock_manager + + result = _supervisor_logs(service, ["worker"]) + + assert result == {"worker": "worker program logs"} + mock_supervisor_manager_class.assert_called_once_with( + str(programs_config_path), service_name="test-service" + ) + mock_manager.get_program_logs.assert_called_once_with("worker") + + +@mock.patch("devservices.commands.logs.SupervisorManager") +def test_supervisor_logs_supervisor_error( + mock_supervisor_manager_class: mock.Mock, + tmp_path: Path, +) -> None: + """Test _supervisor_logs function when supervisor raises an error.""" + # Create a test service + test_service_repo_path = tmp_path / "test-service" + create_mock_git_repo("blank_repo", test_service_repo_path) + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "test-service", + "dependencies": { + "worker": {"description": "Worker", "dependency_type": "supervisor"}, + }, + "modes": {"default": ["worker"]}, + }, + } + create_config_file(test_service_repo_path, config) + + # Create the programs.conf file + programs_config_path = ( + test_service_repo_path / DEVSERVICES_DIR_NAME / PROGRAMS_CONF_FILE_NAME + ) + supervisor_config = """ +[program:worker] +command=python run worker +""" + + create_programs_conf_file(test_service_repo_path, supervisor_config) + + # Load the service from config + from devservices.configs.service_config import load_service_config_from_file + + service_config = load_service_config_from_file(str(test_service_repo_path)) + service = Service( + name=service_config.service_name, + repo_path=str(test_service_repo_path), + config=service_config, + ) + + mock_manager = mock.Mock() + mock_manager.get_program_logs.side_effect = SupervisorError("Failed to get logs") + mock_supervisor_manager_class.return_value = mock_manager + + result = _supervisor_logs(service, ["worker"]) + + assert result == {"worker": "Error getting logs for worker: Failed to get logs"} + mock_supervisor_manager_class.assert_called_once_with( + str(programs_config_path), service_name="test-service" + ) + mock_manager.get_program_logs.assert_called_once_with("worker") diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index 1d9a491b..30786463 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -611,3 +611,110 @@ def test_get_all_process_info_connection_error( # Should return empty list when supervisor is not running assert result == [] + + +@mock.patch("devservices.utils.supervisor.subprocess.run") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_get_program_logs_success( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + """Test successful retrieval of program logs.""" + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } + mock_subprocess_run.return_value = subprocess.CompletedProcess( + args=["supervisorctl", "tail", "test_program"], + returncode=0, + stdout="Program logs output", + stderr="", + ) + + result = supervisor_manager.get_program_logs("test_program") + + assert result == "Program logs output" + mock_subprocess_run.assert_called_once_with( + [ + "supervisorctl", + "-c", + supervisor_manager.config_file_path, + "tail", + "test_program", + ], + capture_output=True, + text=True, + check=True, + ) + + +@mock.patch("devservices.utils.supervisor.subprocess.run") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_get_program_logs_not_running( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + """Test get_program_logs when program is not running.""" + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.STOPPED + } + + result = supervisor_manager.get_program_logs("test_program") + + assert result == "Program test_program is not running" + mock_subprocess_run.assert_not_called() + + +@mock.patch("devservices.utils.supervisor.subprocess.run") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_get_program_logs_failure( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + """Test get_program_logs when subprocess fails.""" + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } + mock_subprocess_run.side_effect = subprocess.CalledProcessError( + 1, "supervisorctl", stderr="Command failed" + ) + + with pytest.raises(SupervisorError, match="Failed to get logs for test_program"): + supervisor_manager.get_program_logs("test_program") + + +@mock.patch("devservices.utils.supervisor.subprocess.run") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_get_program_logs_with_output_and_error( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + """Test get_program_logs returns stdout even when there's stderr.""" + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } + mock_subprocess_run.return_value = subprocess.CompletedProcess( + args=["supervisorctl", "tail", "test_program"], + returncode=0, + stdout="Program logs with warnings", + stderr="Some warnings", + ) + + result = supervisor_manager.get_program_logs("test_program") + + assert result == "Program logs with warnings" + mock_subprocess_run.assert_called_once_with( + [ + "supervisorctl", + "-c", + supervisor_manager.config_file_path, + "tail", + "test_program", + ], + capture_output=True, + text=True, + check=True, + ) From 5010c216f50c8d79245ae159cd76191e7913d2b2 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Wed, 28 May 2025 16:06:15 -0700 Subject: [PATCH 20/23] remove unnecessary comments --- devservices/utils/supervisor.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index a16e305d..572ed267 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -260,7 +260,6 @@ def get_program_logs(self, program_name: str) -> str: return f"Program {program_name} is not running" try: - # Use supervisorctl tail command to get logs result = subprocess.run( [ "supervisorctl", @@ -278,7 +277,6 @@ def get_program_logs(self, program_name: str) -> str: raise SupervisorError(f"Failed to get logs for {program_name}: {str(e)}") def tail_program_logs(self, program_name: str) -> None: - """Tail logs for a supervisor program in real-time (follow mode).""" console = Console() if not self._is_program_running(program_name): From adf957241f926e76894a6e5fdd6eba5813ed3ef3 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Mon, 2 Jun 2025 15:41:17 -0700 Subject: [PATCH 21/23] remove description and group info --- devservices/commands/status.py | 4 ---- tests/commands/test_status.py | 13 +++---------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/devservices/commands/status.py b/devservices/commands/status.py index b0067c97..bd7057bc 100644 --- a/devservices/commands/status.py +++ b/devservices/commands/status.py @@ -429,12 +429,8 @@ def generate_supervisor_status_details( f"Status: {process_info['state_name'].lower()}", f"PID: {process_info['pid'] if process_info['pid'] > 0 else 'N/A'}", f"Uptime: {uptime_str}", - f"Group: {process_info['group'] or 'default'}", ] - if process_info["description"]: - details.append(f"Description: {process_info['description']}") - output.extend(f"{indentation}{BASE_INDENTATION}{detail}" for detail in details) return "\n".join(output) diff --git a/tests/commands/test_status.py b/tests/commands/test_status.py index c70bbeac..9a094bcd 100644 --- a/tests/commands/test_status.py +++ b/tests/commands/test_status.py @@ -890,9 +890,7 @@ def test_generate_supervisor_status_details_running_program() -> None: " Type: program\n" " Status: running\n" " PID: 12345\n" - " Uptime: 1h 1m 1s\n" - " Group: test-group\n" - " Description: Test program description" + " Uptime: 1h 1m 1s" ) @@ -923,8 +921,7 @@ def test_generate_supervisor_status_details_stopped_program() -> None: " Type: program\n" " Status: stopped\n" " PID: N/A\n" - " Uptime: 0s\n" - " Group: default" + " Uptime: 0s" ) @@ -1004,9 +1001,7 @@ def test_generate_service_status_details_supervisor_dependency() -> None: " Type: program\n" " Status: running\n" " PID: 54321\n" - " Uptime: 2m 0s\n" - " Group: supervisor-group\n" - " Description: Test supervisor program" + " Uptime: 2m 0s" ) @@ -1177,7 +1172,5 @@ def test_status_with_supervisor_programs( " Status: running\n" " PID: 12345\n" " Uptime: 1h 0m 0s\n" - " Group: workers\n" - " Description: Background worker process\n" ) assert output == expected_output From 0a7ed1dddd1144a386995951741f67a017df3238 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 3 Jun 2025 12:45:22 -0700 Subject: [PATCH 22/23] better error handling and naming --- devservices/commands/status.py | 35 +++++++------- devservices/utils/supervisor.py | 84 ++++++++++++++++++--------------- tests/commands/test_status.py | 50 ++++++++++---------- tests/utils/test_supervisor.py | 14 +++--- 4 files changed, 94 insertions(+), 89 deletions(-) diff --git a/devservices/commands/status.py b/devservices/commands/status.py index bd7057bc..a2910f84 100644 --- a/devservices/commands/status.py +++ b/devservices/commands/status.py @@ -106,16 +106,16 @@ def status(args: Namespace) -> None: programs_config_path = os.path.join( service.repo_path, f"{DEVSERVICES_DIR_NAME}/{PROGRAMS_CONF_FILE_NAME}" ) - processes_status = [] + process_statuses = {} if os.path.exists(programs_config_path): supervisor_manager = SupervisorManager( programs_config_path, service.name, ) - processes_status = supervisor_manager.get_all_process_info() + process_statuses = supervisor_manager.get_all_process_info() try: - status_tree = get_status_for_service(service, processes_status) + status_tree = get_status_for_service(service, process_statuses) except DependencyError as de: capture_exception(de) console.failure( @@ -130,7 +130,7 @@ def status(args: Namespace) -> None: def get_status_for_service( - service: Service, processes_status: list[ProcessInfo] + service: Service, process_statuses: dict[str, ProcessInfo] ) -> str: state = State() @@ -159,7 +159,7 @@ def get_status_for_service( docker_compose_service_to_status = parse_docker_compose_status(status_json_results) status_tree = generate_service_status_tree( service.name, - processes_status, + process_statuses, dependency_graph, docker_compose_service_to_status, ) @@ -208,7 +208,7 @@ def get_status_json_results( def generate_service_status_tree( service_name: str, - processes_status: list[ProcessInfo], + process_statuses: dict[str, ProcessInfo], dependency_graph: DependencyGraph, docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str = "", @@ -248,7 +248,7 @@ def generate_service_status_tree( output.append( process_service_with_containerized_runtime( dependency, - processes_status, + process_statuses, docker_compose_service_to_status, indentation + BASE_INDENTATION, dependency_graph, @@ -283,7 +283,7 @@ def process_service_with_local_runtime( def process_service_with_containerized_runtime( dependency: DependencyNode, - processes_status: list[ProcessInfo], + process_statuses: dict[str, ProcessInfo], docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str, dependency_graph: DependencyGraph, @@ -291,14 +291,14 @@ def process_service_with_containerized_runtime( if len(dependency_graph.graph[dependency]) > 0: return generate_service_status_tree( dependency.name, - processes_status, + process_statuses, dependency_graph, docker_compose_service_to_status, indentation, ) else: return generate_service_status_details( - dependency, processes_status, docker_compose_service_to_status, indentation + dependency, process_statuses, docker_compose_service_to_status, indentation ) @@ -325,7 +325,7 @@ def parse_docker_compose_status( def generate_service_status_details( dependency: DependencyNode, - processes_status: list[ProcessInfo], + process_statuses: dict[str, ProcessInfo], docker_compose_service_to_status: dict[str, ServiceStatusOutput], indentation: str, ) -> str: @@ -334,7 +334,7 @@ def generate_service_status_details( # Handle supervisor dependencies if dependency.dependency_type == DependencyType.SUPERVISOR: return generate_supervisor_status_details( - dependency, processes_status, indentation + dependency, process_statuses, indentation ) if dependency.name not in docker_compose_service_to_status: @@ -380,7 +380,7 @@ def handle_started_service(dependency: DependencyNode, indentation: str) -> str: f"{indentation}{BASE_INDENTATION}Runtime: local", ] ) - service_output = get_status_for_service(service_with_local_runtime, []) + service_output = get_status_for_service(service_with_local_runtime, {}) return "\n".join( [f"{indentation}{line}" for line in service_output.splitlines()], ) @@ -400,7 +400,7 @@ def format_health(health: str) -> str: def generate_supervisor_status_details( dependency: DependencyNode, - processes_status: list[ProcessInfo], + process_statuses: dict[str, ProcessInfo], indentation: str, ) -> str: """Generate status details for supervisor dependencies.""" @@ -408,10 +408,7 @@ def generate_supervisor_status_details( # Find the specific process in the status list process_info = None - for process_status in processes_status: - if process_status["name"] == dependency.name: - process_info = process_status - break + process_info = process_statuses.get(dependency.name) if process_info is None: return "\n".join( @@ -425,7 +422,7 @@ def generate_supervisor_status_details( uptime_str = format_uptime(process_info["uptime"]) details = [ - "Type: program", + "Type: process", f"Status: {process_info['state_name'].lower()}", f"PID: {process_info['pid'] if process_info['pid'] > 0 else 'N/A'}", f"Uptime: {uptime_str}", diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index 3acf0747..c628cddb 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -284,51 +284,59 @@ def tail_program_logs(self, program_name: str) -> None: except KeyboardInterrupt: pass - def get_all_process_info(self) -> list[ProcessInfo]: + def get_all_process_info(self) -> dict[str, ProcessInfo]: """Get status information for all supervisor programs.""" + # Check if supervisor client is up first, return empty list if down try: client = self._get_rpc_client() + client.supervisor.getState() + except ( + xmlrpc.client.Fault, + SupervisorConnectionError, + socket.error, + ConnectionRefusedError, + ): + return {} + + try: all_process_info = client.supervisor.getAllProcessInfo() # Validate that the response is a list before iterating for typechecking if not isinstance(all_process_info, list): - return [] - - processes_status = [] - for process_info in all_process_info: - if not isinstance(process_info, dict): - continue - - # Extract basic fields with defaults - name = process_info.get("name", "") - state = process_info.get("state", SupervisorProcessState.UNKNOWN) - state_name = SupervisorProcessState(state).name - description = process_info.get("description", "") - pid = process_info.get("pid", 0) - group = process_info.get("group", "") - - # Calculate uptime for running processes - start_time = process_info.get("start", 0) - now = process_info.get("now", 0) - uptime = max(0, now - start_time) if start_time > 0 and now > 0 else 0 - - program_status: ProcessInfo = { - "name": name, - "state": state, - "state_name": state_name, - "description": description, - "pid": pid, - "uptime": uptime, - "start_time": start_time, - "stop_time": process_info.get("stop", 0), - "group": group, - } - processes_status.append(program_status) - - return processes_status + return {} except xmlrpc.client.Fault as e: raise SupervisorError(f"Failed to get programs status: {e.faultString}") - except (SupervisorConnectionError, socket.error, ConnectionRefusedError): - # If supervisor is not running, return empty list - return [] + + processes_status: dict[str, ProcessInfo] = {} + for process_info in all_process_info: + if not isinstance(process_info, dict): + continue + + # Extract basic fields with defaults + name = process_info.get("name", "") + state = process_info.get("state", SupervisorProcessState.UNKNOWN) + state_name = SupervisorProcessState(state).name + description = process_info.get("description", "") + pid = process_info.get("pid", 0) + group = process_info.get("group", "") + + # Calculate uptime for running processes + start_time = process_info.get("start", 0) + now = process_info.get("now", 0) + uptime = max(0, now - start_time) if start_time > 0 and now > 0 else 0 + + program_status: ProcessInfo = { + "name": name, + "state": state, + "state_name": state_name, + "description": description, + "pid": pid, + "uptime": uptime, + "start_time": start_time, + "stop_time": process_info.get("stop", 0), + "group": group, + } + processes_status[name] = program_status + + return processes_status diff --git a/tests/commands/test_status.py b/tests/commands/test_status.py index 9a094bcd..d5b6326a 100644 --- a/tests/commands/test_status.py +++ b/tests/commands/test_status.py @@ -184,7 +184,7 @@ def test_generate_service_status_details() -> None: ], } } - programs_status: list[ProcessInfo] = [] + programs_status: dict[str, ProcessInfo] = {} result = generate_service_status_details( dependency, programs_status, docker_compose_service_to_status, "" ) @@ -206,7 +206,7 @@ def test_generate_service_status_details_missing_status() -> None: dependency_type=DependencyType.SERVICE, ) docker_compose_service_to_status: dict[str, ServiceStatusOutput] = {} - programs_status: list[ProcessInfo] = [] + programs_status: dict[str, ProcessInfo] = {} result = generate_service_status_details( dependency, programs_status, docker_compose_service_to_status, "" ) @@ -256,7 +256,7 @@ def test_generate_service_status_tree_no_child_service( } result = generate_service_status_tree( "parent-service", - [], + {}, dependency_graph, docker_compose_service_to_status, "", @@ -341,7 +341,7 @@ def test_generate_service_status_tree_with_child_service( } result = generate_service_status_tree( "parent-service", - [], + {}, dependency_graph, docker_compose_service_to_status, "", @@ -457,7 +457,7 @@ def test_generate_service_status_tree_with_nested_child_services( } result = generate_service_status_tree( "grandparent-service", - [], + {}, dependency_graph, docker_compose_service_to_status, "", @@ -869,8 +869,8 @@ def test_generate_supervisor_status_details_running_program() -> None: name="test-program", dependency_type=DependencyType.SUPERVISOR, ) - programs_status: list[ProcessInfo] = [ - { + programs_status: dict[str, ProcessInfo] = { + "test-program": { "name": "test-program", "state": SupervisorProcessState.RUNNING, # RUNNING "state_name": SupervisorProcessState.RUNNING.name, @@ -881,13 +881,13 @@ def test_generate_supervisor_status_details_running_program() -> None: "stop_time": 0, "group": "test-group", } - ] + } result = generate_supervisor_status_details(dependency, programs_status, "") assert result == ( f"{Color.BOLD}test-program{Color.RESET}:\n" - " Type: program\n" + " Type: process\n" " Status: running\n" " PID: 12345\n" " Uptime: 1h 1m 1s" @@ -900,8 +900,8 @@ def test_generate_supervisor_status_details_stopped_program() -> None: name="stopped-program", dependency_type=DependencyType.SUPERVISOR, ) - programs_status: list[ProcessInfo] = [ - { + programs_status: dict[str, ProcessInfo] = { + "stopped-program": { "name": "stopped-program", "state": SupervisorProcessState.STOPPED, # STOPPED "state_name": SupervisorProcessState.STOPPED.name, @@ -912,13 +912,13 @@ def test_generate_supervisor_status_details_stopped_program() -> None: "stop_time": 1234567890, "group": "", } - ] + } result = generate_supervisor_status_details(dependency, programs_status, " ") assert result == ( f" {Color.BOLD}stopped-program{Color.RESET}:\n" - " Type: program\n" + " Type: process\n" " Status: stopped\n" " PID: N/A\n" " Uptime: 0s" @@ -931,8 +931,8 @@ def test_generate_supervisor_status_details_program_not_found() -> None: name="missing-program", dependency_type=DependencyType.SUPERVISOR, ) - programs_status: list[ProcessInfo] = [ - { + programs_status: dict[str, ProcessInfo] = { + "other-program": { "name": "other-program", "state": SupervisorProcessState.RUNNING, "state_name": SupervisorProcessState.RUNNING.name, @@ -943,7 +943,7 @@ def test_generate_supervisor_status_details_program_not_found() -> None: "stop_time": 0, "group": "test", } - ] + } result = generate_supervisor_status_details(dependency, programs_status, "") @@ -960,7 +960,7 @@ def test_generate_supervisor_status_details_empty_programs_list() -> None: name="test-program", dependency_type=DependencyType.SUPERVISOR, ) - programs_status: list[ProcessInfo] = [] + programs_status: dict[str, ProcessInfo] = {} result = generate_supervisor_status_details(dependency, programs_status, "") @@ -977,8 +977,8 @@ def test_generate_service_status_details_supervisor_dependency() -> None: name="test-supervisor-program", dependency_type=DependencyType.SUPERVISOR, ) - programs_status: list[ProcessInfo] = [ - { + programs_status: dict[str, ProcessInfo] = { + "test-supervisor-program": { "name": "test-supervisor-program", "state": SupervisorProcessState.RUNNING, "state_name": SupervisorProcessState.RUNNING.name, @@ -989,7 +989,7 @@ def test_generate_service_status_details_supervisor_dependency() -> None: "stop_time": 0, "group": "supervisor-group", } - ] + } docker_compose_service_to_status: dict[str, ServiceStatusOutput] = {} result = generate_service_status_details( @@ -998,7 +998,7 @@ def test_generate_service_status_details_supervisor_dependency() -> None: assert result == ( f"{Color.BOLD}test-supervisor-program{Color.RESET}:\n" - " Type: program\n" + " Type: process\n" " Status: running\n" " PID: 54321\n" " Uptime: 2m 0s" @@ -1109,8 +1109,8 @@ def test_status_with_supervisor_programs( run_git_command(["commit", "-m", "Add config"], cwd=test_service_repo_path) # Mock supervisor programs status - mock_programs_status: list[ProcessInfo] = [ - { + mock_programs_status: dict[str, ProcessInfo] = { + "worker": { "name": "worker", "state": SupervisorProcessState.RUNNING, "state_name": "RUNNING", @@ -1121,7 +1121,7 @@ def test_status_with_supervisor_programs( "stop_time": 0, "group": "workers", }, - ] + } # Mock docker compose status for clickhouse (matching the config) mock_docker_status = [ @@ -1168,7 +1168,7 @@ def test_status_with_supervisor_programs( " Ports:\n" " 127.0.0.1:9000 -> 9000/tcp\n" f" {Color.BOLD}worker{Color.RESET}:\n" - " Type: program\n" + " Type: process\n" " Status: running\n" " PID: 12345\n" " Uptime: 1h 0m 0s\n" diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index 1d9a491b..67e1a42a 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -546,8 +546,8 @@ def test_get_all_process_info_success( assert len(result) == 2 - expected_results = [ - { + expected_results = { + "program1": { "name": "program1", "state": SupervisorProcessState.RUNNING, "state_name": "RUNNING", @@ -558,7 +558,7 @@ def test_get_all_process_info_success( "start_time": 1000, "stop_time": 0, }, - { + "program2": { "name": "program2", "state": SupervisorProcessState.STOPPED, "state_name": "STOPPED", @@ -569,7 +569,7 @@ def test_get_all_process_info_success( "start_time": 0, "stop_time": 1050, }, - ] + } for expected, actual in zip(expected_results, result): assert actual == expected @@ -584,7 +584,7 @@ def test_get_all_process_info_empty_list( result = supervisor_manager.get_all_process_info() - assert result == [] + assert result == {} @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") @@ -609,5 +609,5 @@ def test_get_all_process_info_connection_error( result = supervisor_manager.get_all_process_info() - # Should return empty list when supervisor is not running - assert result == [] + # Should return empty dict when supervisor is not running + assert result == {} From 29de7a748a03cd0395e2956846751350e425ad3a Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Tue, 3 Jun 2025 14:21:54 -0700 Subject: [PATCH 23/23] multithread supervisor logs, better exception handling, and naming --- devservices/commands/logs.py | 38 ++++++++++++++++++++++----------- devservices/utils/supervisor.py | 2 -- tests/commands/test_logs.py | 12 +++++++---- tests/utils/test_supervisor.py | 18 ---------------- 4 files changed, 34 insertions(+), 36 deletions(-) diff --git a/devservices/commands/logs.py b/devservices/commands/logs.py index 3bb7df1b..490478c4 100644 --- a/devservices/commands/logs.py +++ b/devservices/commands/logs.py @@ -22,6 +22,7 @@ from devservices.exceptions import DependencyError from devservices.exceptions import DockerComposeError from devservices.exceptions import ServiceNotFoundError +from devservices.exceptions import SupervisorConfigError from devservices.exceptions import SupervisorError from devservices.utils.console import Console from devservices.utils.dependencies import install_and_verify_dependencies @@ -121,7 +122,7 @@ def logs(args: Namespace) -> None: == DependencyType.SUPERVISOR ] - if supervisor_programs: + if len(supervisor_programs) > 0: supervisor_logs = _supervisor_logs(service, supervisor_programs) for program_name, log_content in supervisor_logs.items(): if log_content: @@ -175,23 +176,36 @@ def _supervisor_logs( if not supervisor_programs: return {} - supervisor_logs = {} + supervisor_logs: dict[str, str] = {} programs_config_path = os.path.join( service.repo_path, DEVSERVICES_DIR_NAME, PROGRAMS_CONF_FILE_NAME ) - manager = SupervisorManager(programs_config_path, service_name=service.name) + try: + manager = SupervisorManager(programs_config_path, service_name=service.name) + except SupervisorConfigError as e: + capture_exception(e) + return supervisor_logs - for program_name in supervisor_programs: - try: - log_content = manager.get_program_logs(program_name) + with concurrent.futures.ThreadPoolExecutor() as executor: + futures = [ + executor.submit(get_program_logs_with_error_handling, manager, program_name) + for program_name in supervisor_programs + ] + for future in concurrent.futures.as_completed(futures): + program_name, log_content = future.result() supervisor_logs[program_name] = log_content - except SupervisorError as e: - # Log the error but continue with other programs - capture_exception(e) - supervisor_logs[ - program_name - ] = f"Error getting logs for {program_name}: {str(e)}" return supervisor_logs + + +def get_program_logs_with_error_handling( + manager: SupervisorManager, program_name: str +) -> tuple[str, str]: + try: + log_content = manager.get_program_logs(program_name) + return program_name, log_content + except SupervisorError as e: + capture_exception(e) + return program_name, f"Error getting logs for {program_name}: {str(e)}" diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index 4e4015f8..8386253c 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -262,8 +262,6 @@ def get_program_command(self, program_name: str) -> str: def get_program_logs(self, program_name: str) -> str: """Get logs for a supervisor program as text output.""" - if not self._is_program_running(program_name): - return f"Program {program_name} is not running" try: result = subprocess.run( diff --git a/tests/commands/test_logs.py b/tests/commands/test_logs.py index ac437f7e..388bd5f7 100644 --- a/tests/commands/test_logs.py +++ b/tests/commands/test_logs.py @@ -11,6 +11,7 @@ from devservices.commands.logs import _supervisor_logs from devservices.commands.logs import logs from devservices.configs.service_config import load_service_config_from_file +from devservices.constants import Color from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEVSERVICES_DIR_NAME from devservices.constants import PROGRAMS_CONF_FILE_NAME @@ -87,7 +88,10 @@ def test_logs_no_specified_service_not_running( mock_get_docker_compose_commands_to_run.assert_not_called() captured = capsys.readouterr() - assert "\x1b[0;33mService test-service is not running\x1b[0m\n" == captured.out + assert ( + f"{Color.YELLOW}Service test-service is not running{Color.RESET}\n" + == captured.out + ) @mock.patch("devservices.commands.logs.run_cmd") @@ -250,7 +254,7 @@ def test_logs_service_not_found_error( captured = capsys.readouterr() assert ( - "\x1b[0;31mService 'nonexistent-service' not found.\x1b[0m" + f"{Color.RED}Service 'nonexistent-service' not found.{Color.RESET}" == captured.out.strip() ) @@ -305,7 +309,7 @@ def test_logs_dependency_error( captured = capsys.readouterr() assert ( - f"\x1b[0;31mDependencyError: test-service ({tmp_path}) on main. If this error persists, try running `devservices purge`\x1b[0m\n" + f"{Color.RED}DependencyError: test-service ({tmp_path}) on main. If this error persists, try running `devservices purge`{Color.RESET}\n" == captured.out ) @@ -363,7 +367,7 @@ def test_logs_docker_compose_error( captured = capsys.readouterr() assert ( - "\x1b[0;31mFailed to get logs for test-service: stderr_output\x1b[0m\n" + f"{Color.RED}Failed to get logs for test-service: stderr_output{Color.RESET}\n" == captured.out ) diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index c663325b..1b9e0b49 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -648,24 +648,6 @@ def test_get_program_logs_success( ) -@mock.patch("devservices.utils.supervisor.subprocess.run") -@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_get_program_logs_not_running( - mock_rpc_client: mock.MagicMock, - mock_subprocess_run: mock.MagicMock, - supervisor_manager: SupervisorManager, -) -> None: - """Test get_program_logs when program is not running.""" - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { - "state": SupervisorProcessState.STOPPED - } - - result = supervisor_manager.get_program_logs("test_program") - - assert result == "Program test_program is not running" - mock_subprocess_run.assert_not_called() - - @mock.patch("devservices.utils.supervisor.subprocess.run") @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") def test_get_program_logs_failure(