From 5a82809630a665219cba15a85d57d681b05e7087 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Mon, 12 May 2025 16:58:16 -0700 Subject: [PATCH 01/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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 a3605bce19d440210cb1450982cda93cdb42ba2a Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Mon, 2 Jun 2025 19:30:51 -0700 Subject: [PATCH 16/17] address some feedback --- devservices/commands/foreground.py | 36 +++-- tests/commands/test_foreground.py | 212 ++++++++++++++++++++++++----- 2 files changed, 199 insertions(+), 49 deletions(-) diff --git a/devservices/commands/foreground.py b/devservices/commands/foreground.py index 78e98ed9..737a8076 100644 --- a/devservices/commands/foreground.py +++ b/devservices/commands/foreground.py @@ -16,6 +16,7 @@ from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import ServiceNotFoundError from devservices.exceptions import SupervisorConfigError +from devservices.exceptions import SupervisorProcessError from devservices.utils.console import Console from devservices.utils.services import find_matching_service from devservices.utils.state import State @@ -24,7 +25,9 @@ def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: - parser = subparsers.add_parser("foreground", help="Run a service in the foreground") + parser = subparsers.add_parser( + "foreground", help="Run a service's program in the foreground" + ) parser.add_argument( "program_name", help="Name of the program to run in the foreground" ) @@ -32,7 +35,7 @@ def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: def foreground(args: Namespace) -> None: - """Run a service in the foreground.""" + """Run a service's program in the foreground.""" console = Console() program_name = args.program_name try: @@ -51,13 +54,18 @@ def foreground(args: Namespace) -> None: console.failure(str(e)) exit(1) modes = service.config.modes + if program_name not in service.config.dependencies: + console.failure( + f"Program {program_name} does not exist in the service's config" + ) + return 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 # + return active_starting_modes = state.get_active_modes_for_service( service.name, StateTables.STARTING_SERVICES ) @@ -79,7 +87,9 @@ def foreground(args: Namespace) -> None: ] if program_name not in supervisor_programs: - console.failure(f"Program {program_name} is not currently running.") + console.failure( + f"Program {program_name} is not running in any active modes of {service.name}" + ) return programs_config_path = os.path.join( @@ -102,20 +112,22 @@ def foreground(args: Namespace) -> None: # 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: + except SupervisorProcessError as e: + capture_exception(e) + console.failure(f"Error stopping {program_name} in supervisor: {str(e)}") + except (OSError, FileNotFoundError, PermissionError) 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 + + try: + console.info(f"Restarting {program_name} in background") manager.start_process(program_name) - return + except SupervisorProcessError as e: + capture_exception(e) + console.failure(f"Error restarting {program_name} in background: {str(e)}") diff --git a/tests/commands/test_foreground.py b/tests/commands/test_foreground.py index d95ae9c2..737643e3 100644 --- a/tests/commands/test_foreground.py +++ b/tests/commands/test_foreground.py @@ -13,6 +13,7 @@ from devservices.exceptions import ConfigError from devservices.exceptions import ServiceNotFoundError from devservices.exceptions import SupervisorConfigError +from devservices.exceptions import SupervisorProcessError from devservices.utils.state import State from devservices.utils.state import StateTables from testing.utils import create_config_file @@ -104,7 +105,7 @@ def test_foreground_service_not_running( foreground(args) captured = capsys.readouterr() - assert "example-service is not running" in captured.out + assert "\x1b[0;33mexample-service is not running\x1b[0m\n" == captured.out mock_pty_spawn.assert_not_called() @@ -152,7 +153,61 @@ def test_foreground_program_not_in_supervisor_programs( foreground(args) captured = capsys.readouterr() - assert "Program nonexistent is not currently running." in captured.out + assert ( + "\x1b[0;31mProgram nonexistent does not exist in the service's config\x1b[0m\n" + == captured.out + ) + + mock_pty_spawn.assert_not_called() + + +@mock.patch("devservices.commands.foreground.pty.spawn") +def test_foreground_program_not_in_active_modes( + 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"], "other": ["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) + + captured = capsys.readouterr() + assert ( + "\x1b[0;31mProgram worker is not running in any active modes of example-service\x1b[0m\n" + == captured.out + ) mock_pty_spawn.assert_not_called() @@ -339,7 +394,7 @@ def test_foreground_pty_spawn_exception( """ create_programs_conf_file(service_path, programs_config) - mock_pty_spawn.side_effect = Exception("Spawn failed") + mock_pty_spawn.side_effect = OSError("Spawn failed") args = Namespace(program_name="worker") @@ -352,7 +407,7 @@ def test_foreground_pty_spawn_exception( 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" + "Stopping worker in supervisor\nStarting worker in foreground\n\x1b[0;31mError running worker in foreground: Spawn failed\x1b[0m\nRestarting worker in background\n" == captured.out ) @@ -363,11 +418,12 @@ def test_foreground_pty_spawn_exception( @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( +def test_foreground_stop_process_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": { @@ -392,49 +448,46 @@ def test_foreground_with_starting_services( """ create_programs_conf_file(service_path, programs_config) + mock_stop_process.side_effect = SupervisorProcessError("Stop process 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.STARTING_SERVICES + "example-service", "default", StateTables.STARTED_SERVICES ) foreground(args) - # Verify calls - mock_pty_spawn.assert_called_once_with(["python", "worker.py"]) - + mock_pty_spawn.assert_not_called() mock_start_process.assert_called_once_with("worker") mock_stop_process.assert_called_once_with("worker") + captured = capsys.readouterr() + assert ( + "Stopping worker in supervisor\n\x1b[0;31mError stopping worker in supervisor: Stop process failed\x1b[0m\nRestarting worker in background\n" + == captured.out + ) + @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( +def test_foreground_start_process_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": { - "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"}, + "modes": {"default": ["worker"]}, }, } @@ -447,38 +500,40 @@ def test_foreground_multiple_modes_and_dependencies( 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") + mock_start_process.side_effect = SupervisorProcessError("Start process 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", "full", StateTables.STARTED_SERVICES + "example-service", "default", StateTables.STARTED_SERVICES ) foreground(args) - # Verify calls - mock_pty_spawn.assert_called_once_with(["python", "consumer.py"]) + 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_start_process.assert_called_once_with("consumer") - mock_stop_process.assert_called_once_with("consumer") + captured = capsys.readouterr() + assert ( + "Stopping worker in supervisor\nStarting worker in foreground\nRestarting worker in background\n\x1b[0;31mError restarting worker in background: Start process failed\x1b[0m\n" + == captured.out + ) @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( +def test_foreground_with_starting_services( 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": { @@ -497,7 +552,7 @@ def test_foreground_empty_command( programs_config = """ [program:worker] -command= +command=python worker.py autostart=true autorestart=true """ @@ -508,21 +563,98 @@ def test_foreground_empty_command( 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 + "example-service", "default", StateTables.STARTING_SERVICES ) foreground(args) - # Verify that shlex.split handles empty command correctly - mock_pty_spawn.assert_called_once_with([]) + # 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") + captured = capsys.readouterr() + assert ( + "Stopping worker in supervisor\nStarting worker in foreground\nRestarting worker in background\n" + == captured.out + ) + + +@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, + capsys: pytest.CaptureFixture[str], +) -> 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") + + captured = capsys.readouterr() + assert ( + "Stopping consumer in supervisor\nStarting consumer in foreground\nRestarting consumer in background\n" + == captured.out + ) + @mock.patch("devservices.commands.foreground.pty.spawn") def test_foreground_no_active_modes( mock_pty_spawn: mock.Mock, tmp_path: Path, + capsys: pytest.CaptureFixture[str], ) -> None: config = { "x-sentry-service-config": { @@ -558,3 +690,9 @@ def test_foreground_no_active_modes( # Should not call pty.spawn since no supervisor programs are active mock_pty_spawn.assert_not_called() + + captured = capsys.readouterr() + assert ( + captured.out + == "\x1b[0;31mProgram worker is not running in any active modes of example-service\x1b[0m\n" + ) From 37aa3373dae67170c4d529298e60f6f71d1ac198 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Mon, 2 Jun 2025 19:44:20 -0700 Subject: [PATCH 17/17] use color constants --- tests/commands/test_foreground.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/commands/test_foreground.py b/tests/commands/test_foreground.py index 737643e3..e0c43007 100644 --- a/tests/commands/test_foreground.py +++ b/tests/commands/test_foreground.py @@ -8,6 +8,7 @@ import pytest from devservices.commands.foreground import foreground +from devservices.constants import Color from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEVSERVICES_DIR_NAME from devservices.exceptions import ConfigError @@ -105,7 +106,9 @@ def test_foreground_service_not_running( foreground(args) captured = capsys.readouterr() - assert "\x1b[0;33mexample-service is not running\x1b[0m\n" == captured.out + assert ( + f"{Color.YELLOW}example-service is not running{Color.RESET}\n" == captured.out + ) mock_pty_spawn.assert_not_called() @@ -154,7 +157,7 @@ def test_foreground_program_not_in_supervisor_programs( captured = capsys.readouterr() assert ( - "\x1b[0;31mProgram nonexistent does not exist in the service's config\x1b[0m\n" + f"{Color.RED}Program nonexistent does not exist in the service's config{Color.RESET}\n" == captured.out ) @@ -205,7 +208,7 @@ def test_foreground_program_not_in_active_modes( captured = capsys.readouterr() assert ( - "\x1b[0;31mProgram worker is not running in any active modes of example-service\x1b[0m\n" + f"{Color.RED}Program worker is not running in any active modes of example-service{Color.RESET}\n" == captured.out ) @@ -251,7 +254,7 @@ def test_foreground_programs_conf_not_found( 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" + f"{Color.RED}Dependency 'worker' is not remote but is not defined in docker-compose services or programs file{Color.RESET}\n" == captured.out ) @@ -271,7 +274,7 @@ def test_foreground_config_not_found_error( 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" + f"{Color.RED}No 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.{Color.RESET}\n" == captured.out ) @@ -289,7 +292,7 @@ def test_foreground_config_error( foreground(args) captured = capsys.readouterr() - assert "\x1b[0;31mInvalid config\x1b[0m\n" == captured.out + assert f"{Color.RED}Invalid config{Color.RESET}\n" == captured.out @mock.patch("devservices.commands.foreground.find_matching_service") @@ -305,7 +308,7 @@ def test_foreground_service_not_found_error( foreground(args) captured = capsys.readouterr() - assert "\x1b[0;31mService not found\x1b[0m\n" == captured.out + assert f"{Color.RED}Service not found{Color.RESET}\n" == captured.out @mock.patch("devservices.commands.foreground.pty.spawn") @@ -353,7 +356,7 @@ def test_foreground_supervisor_config_error( # Verify output captured = capsys.readouterr() assert ( - "\x1b[0;31mError when getting program command: Program config error\x1b[0m\n" + f"{Color.RED}Error when getting program command: Program config error{Color.RESET}\n" == captured.out ) @@ -407,7 +410,7 @@ def test_foreground_pty_spawn_exception( captured = capsys.readouterr() assert ( - "Stopping worker in supervisor\nStarting worker in foreground\n\x1b[0;31mError running worker in foreground: Spawn failed\x1b[0m\nRestarting worker in background\n" + f"Stopping worker in supervisor\nStarting worker in foreground\n{Color.RED}Error running worker in foreground: Spawn failed{Color.RESET}\nRestarting worker in background\n" == captured.out ) @@ -465,7 +468,7 @@ def test_foreground_stop_process_exception( captured = capsys.readouterr() assert ( - "Stopping worker in supervisor\n\x1b[0;31mError stopping worker in supervisor: Stop process failed\x1b[0m\nRestarting worker in background\n" + f"Stopping worker in supervisor\n{Color.RED}Error stopping worker in supervisor: Stop process failed{Color.RESET}\nRestarting worker in background\n" == captured.out ) @@ -520,7 +523,7 @@ def test_foreground_start_process_exception( captured = capsys.readouterr() assert ( - "Stopping worker in supervisor\nStarting worker in foreground\nRestarting worker in background\n\x1b[0;31mError restarting worker in background: Start process failed\x1b[0m\n" + f"Stopping worker in supervisor\nStarting worker in foreground\nRestarting worker in background\n{Color.RED}Error restarting worker in background: Start process failed{Color.RESET}\n" == captured.out ) @@ -694,5 +697,5 @@ def test_foreground_no_active_modes( captured = capsys.readouterr() assert ( captured.out - == "\x1b[0;31mProgram worker is not running in any active modes of example-service\x1b[0m\n" + == f"{Color.RED}Program worker is not running in any active modes of example-service{Color.RESET}\n" )