From da94595965f473bd657a1665daaca654f6c7248b Mon Sep 17 00:00:00 2001 From: ash Date: Fri, 16 Feb 2024 14:27:29 +0000 Subject: [PATCH 1/4] Add /resend-pending-builds endpoint --- softpack_core/schemas/environment.py | 35 ++++++++++++++----- softpack_core/service.py | 12 +++++++ tests/integration/test_environment.py | 33 +++--------------- tests/integration/test_resend_builds.py | 46 +++++++++++++++++++++++++ tests/integration/utils.py | 26 ++++++++++++++ 5 files changed, 114 insertions(+), 38 deletions(-) create mode 100644 tests/integration/test_resend_builds.py diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 5c6b8e1..0a31ec6 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -292,15 +292,13 @@ def iter(cls) -> Iterable["Environment"]: ) for env in environment_objects: + env.avg_wait_secs = avg_wait_secs status = status_map.get(str(Path(env.path, env.name))) if not status: - if env.state == State.queued: - env.state = State.failed continue env.requested = status.requested env.build_start = status.build_start env.build_done = status.build_done - env.avg_wait_secs = avg_wait_secs return environment_objects @@ -360,7 +358,30 @@ def create(cls, env: EnvironmentInput) -> CreateResponse: # type: ignore version += 1 - # Send build request + response = cls.submit_env_to_builder(env) + if response is not None: + cls.delete(env.name, env.path) + return response + + return CreateEnvironmentSuccess( + message="Successfully scheduled environment creation" + ) + + @classmethod + def submit_env_to_builder( + cls, env: EnvironmentInput + ) -> Union[None, BuilderError, InvalidInputError]: + """Submit an environment to the builder.""" + try: + m = re.fullmatch(r"^(.*)-(\d+)$", env.name) + if not m: + raise Exception + versionless_name, version = m.groups() + except Exception: + return InvalidInputError( + message=f"could not parse version from name: {env.name!r}" + ) + try: host = app.settings.builder.host port = app.settings.builder.port @@ -383,15 +404,11 @@ def create(cls, env: EnvironmentInput) -> CreateResponse: # type: ignore ) r.raise_for_status() except Exception as e: - cls.delete(env.name, env.path) return BuilderError( message="Connection to builder failed: " + "".join(format_exception_only(type(e), e)) ) - - return CreateEnvironmentSuccess( - message="Successfully scheduled environment creation" - ) + return None @classmethod def create_new_env( diff --git a/softpack_core/service.py b/softpack_core/service.py index 6ea3829..d8032f1 100644 --- a/softpack_core/service.py +++ b/softpack_core/service.py @@ -14,6 +14,7 @@ from typer import Typer from typing_extensions import Annotated +from softpack_core.artifacts import State from softpack_core.schemas.environment import ( CreateEnvironmentSuccess, Environment, @@ -92,3 +93,14 @@ async def upload_artifacts( # type: ignore[no-untyped-def] raise Exception(resp) return resp + + @staticmethod + @router.post("/resend-pending-builds") + async def resend_pending_builds(): # type: ignore[no-untyped-def] + """Resubmit any pending builds to the builder.""" + for env in Environment.iter(): + if env.state != State.queued: + continue + Environment.submit_env_to_builder(env) + + return {"message": "Successfully triggered resend"} diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index fcd0721..fca697c 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -14,7 +14,7 @@ import yaml from fastapi import UploadFile -from softpack_core.artifacts import Artifacts, app +from softpack_core.artifacts import Artifacts from softpack_core.schemas.environment import ( BuilderError, CreateEnvironmentSuccess, @@ -29,7 +29,7 @@ UpdateEnvironmentSuccess, WriteArtifactSuccess, ) -from tests.integration.utils import file_in_remote +from tests.integration.utils import builder_called_correctly, file_in_remote pytestmark = pytest.mark.repo @@ -160,31 +160,6 @@ def test_create_cleans_up_after_builder_failure( assert not file_in_remote(ymlPath) -def builder_called_correctly( - post_mock, testable_env_input: EnvironmentInput -) -> None: - # TODO: don't mock this; actually have a real builder service to test with? - host = app.settings.builder.host - port = app.settings.builder.port - post_mock.assert_called_with( - f"http://{host}:{port}/environments/build", - json={ - "name": f"{testable_env_input.path}/{testable_env_input.name}", - "version": "1", - "model": { - "description": testable_env_input.description, - "packages": [ - { - "name": pkg.name, - "version": pkg.version, - } - for pkg in testable_env_input.packages - ], - }, - }, - ) - - def test_delete(httpx_post, testable_env_input) -> None: result = Environment.delete( testable_env_input.name, testable_env_input.path @@ -307,8 +282,8 @@ def test_iter_no_statuses(testable_env_input, mocker): assert envs[0].build_start is None assert envs[0].build_done is None assert envs[0].avg_wait_secs is None - assert envs[0].state == State.failed - assert envs[1].state == State.failed + assert envs[0].state == State.queued + assert envs[1].state == State.queued @pytest.mark.asyncio diff --git a/tests/integration/test_resend_builds.py b/tests/integration/test_resend_builds.py new file mode 100644 index 0000000..056fcce --- /dev/null +++ b/tests/integration/test_resend_builds.py @@ -0,0 +1,46 @@ +"""Copyright (c) 2024 Genome Research Ltd. + +This source code is licensed under the MIT license found in the +LICENSE file in the root directory of this source tree. +""" + + +import pytest +from fastapi.testclient import TestClient + +from softpack_core.app import app +from softpack_core.schemas.environment import ( + CreateEnvironmentSuccess, + Environment, + EnvironmentInput, +) +from softpack_core.service import ServiceAPI +from tests.integration.utils import builder_called_correctly + +pytestmark = pytest.mark.repo + + +def test_resend_pending_builds( + httpx_post, testable_env_input: EnvironmentInput +): + ServiceAPI.register() + client = TestClient(app.router) + + orig_name = testable_env_input.name + testable_env_input.name += "-1" + r = Environment.create_new_env( + testable_env_input, Environment.artifacts.built_by_softpack_file + ) + assert isinstance(r, CreateEnvironmentSuccess) + testable_env_input.name = orig_name + + httpx_post.assert_not_called() + + resp = client.post( + url="/resend-pending-builds", + ) + assert resp.status_code == 200 + assert resp.json().get("message") == "Successfully triggered resend" + + httpx_post.assert_called_once() + builder_called_correctly(httpx_post, testable_env_input) diff --git a/tests/integration/utils.py b/tests/integration/utils.py index dd82250..31b2e16 100644 --- a/tests/integration/utils.py +++ b/tests/integration/utils.py @@ -12,6 +12,7 @@ import pytest from softpack_core.artifacts import Artifacts, app +from softpack_core.schemas.environment import EnvironmentInput artifacts_dict = dict[ str, @@ -199,3 +200,28 @@ def file_in_repo( current = current[part] return current + + +def builder_called_correctly( + post_mock, testable_env_input: EnvironmentInput +) -> None: + # TODO: don't mock this; actually have a real builder service to test with? + host = app.settings.builder.host + port = app.settings.builder.port + post_mock.assert_called_with( + f"http://{host}:{port}/environments/build", + json={ + "name": f"{testable_env_input.path}/{testable_env_input.name}", + "version": "1", + "model": { + "description": testable_env_input.description, + "packages": [ + { + "name": pkg.name, + "version": pkg.version, + } + for pkg in testable_env_input.packages + ], + }, + }, + ) From 2558a6f7e7ec6b64b6fd9a52be147b2c7eee2f4d Mon Sep 17 00:00:00 2001 From: ash Date: Tue, 20 Feb 2024 14:09:13 +0000 Subject: [PATCH 2/4] Queue builds even when builder is unavailable --- softpack_core/schemas/environment.py | 1 - 1 file changed, 1 deletion(-) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 0a31ec6..5e0f9ae 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -360,7 +360,6 @@ def create(cls, env: EnvironmentInput) -> CreateResponse: # type: ignore response = cls.submit_env_to_builder(env) if response is not None: - cls.delete(env.name, env.path) return response return CreateEnvironmentSuccess( From d3e1f21a74119a9a28332a50cfdc4d1d410d88c3 Mon Sep 17 00:00:00 2001 From: ash Date: Tue, 20 Feb 2024 14:19:14 +0000 Subject: [PATCH 3/4] Fix tests --- tests/integration/test_environment.py | 6 +++--- tests/integration/test_spack.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index fca697c..478fd9c 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -142,7 +142,7 @@ def test_create_path_invalid_disallowed(httpx_post, testable_env_input, path): assert isinstance(result, InvalidInputError) -def test_create_cleans_up_after_builder_failure( +def test_create_does_not_clean_up_after_builder_failure( httpx_post, testable_env_input ): httpx_post.side_effect = Exception('could not contact builder') @@ -156,8 +156,8 @@ def test_create_cleans_up_after_builder_failure( ) builtPath = dir / Environment.artifacts.built_by_softpack_file ymlPath = dir / Environment.artifacts.environments_file - assert not file_in_remote(builtPath) - assert not file_in_remote(ymlPath) + assert file_in_remote(builtPath) + assert file_in_remote(ymlPath) def test_delete(httpx_post, testable_env_input) -> None: diff --git a/tests/integration/test_spack.py b/tests/integration/test_spack.py index 3baf616..299a4a6 100644 --- a/tests/integration/test_spack.py +++ b/tests/integration/test_spack.py @@ -45,7 +45,7 @@ def test_spack_packages(): else: assert len(packages) > len(pkgs) - spack = Spack(custom_repo = app.settings.spack.repo) + spack = Spack(custom_repo=app.settings.spack.repo) spack.packages() From e2819fac675e736e667550f7854d9c7f8360baef Mon Sep 17 00:00:00 2001 From: ash Date: Wed, 21 Feb 2024 14:22:11 +0000 Subject: [PATCH 4/4] Return number of successful/failed resends from /resend-pending-builds --- softpack_core/service.py | 32 +++++++++++++++++++++---- tests/integration/test_resend_builds.py | 15 +++++++++++- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/softpack_core/service.py b/softpack_core/service.py index d8032f1..e9f11ab 100644 --- a/softpack_core/service.py +++ b/softpack_core/service.py @@ -10,7 +10,7 @@ import typer import uvicorn -from fastapi import APIRouter, Request, UploadFile +from fastapi import APIRouter, Request, Response, UploadFile from typer import Typer from typing_extensions import Annotated @@ -19,6 +19,7 @@ CreateEnvironmentSuccess, Environment, EnvironmentInput, + PackageInput, WriteArtifactSuccess, ) @@ -96,11 +97,32 @@ async def upload_artifacts( # type: ignore[no-untyped-def] @staticmethod @router.post("/resend-pending-builds") - async def resend_pending_builds(): # type: ignore[no-untyped-def] + async def resend_pending_builds( # type: ignore[no-untyped-def] + response: Response, + ): """Resubmit any pending builds to the builder.""" + successes = 0 + failures = 0 for env in Environment.iter(): if env.state != State.queued: continue - Environment.submit_env_to_builder(env) - - return {"message": "Successfully triggered resend"} + result = Environment.submit_env_to_builder( + EnvironmentInput( + name=env.name, + path=env.path, + description=env.description, + packages=[PackageInput(**vars(p)) for p in env.packages], + ) + ) + if result is None: + successes += 1 + else: + failures += 1 + + if failures == 0: + message = "Successfully triggered resends" + else: + response.status_code = 500 + message = "Failed to trigger all resends" + + return {"message": message, "successes": successes, "failures": failures} diff --git a/tests/integration/test_resend_builds.py b/tests/integration/test_resend_builds.py index 056fcce..bfe76c5 100644 --- a/tests/integration/test_resend_builds.py +++ b/tests/integration/test_resend_builds.py @@ -23,6 +23,8 @@ def test_resend_pending_builds( httpx_post, testable_env_input: EnvironmentInput ): + Environment.delete("test_environment", "users/test_user") + Environment.delete("test_environment", "groups/test_group") ServiceAPI.register() client = TestClient(app.router) @@ -40,7 +42,18 @@ def test_resend_pending_builds( url="/resend-pending-builds", ) assert resp.status_code == 200 - assert resp.json().get("message") == "Successfully triggered resend" + assert resp.json().get("message") == "Successfully triggered resends" + assert resp.json().get("successes") == 1 + assert resp.json().get("failures") == 0 httpx_post.assert_called_once() builder_called_correctly(httpx_post, testable_env_input) + + httpx_post.side_effect = Exception('could not contact builder') + resp = client.post( + url="/resend-pending-builds", + ) + assert resp.status_code == 500 + assert resp.json().get("message") == "Failed to trigger all resends" + assert resp.json().get("successes") == 0 + assert resp.json().get("failures") == 1