diff --git a/bclaw_runner/src/runner/qc_check.py b/bclaw_runner/src/runner/qc_check.py index 3b00787..66cb34b 100644 --- a/bclaw_runner/src/runner/qc_check.py +++ b/bclaw_runner/src/runner/qc_check.py @@ -1,40 +1,41 @@ import json import logging import os +from typing import Generator import boto3 logger = logging.getLogger(__name__) -def run_qc_checks(checks: list) -> None: - if checks: - logger.info("starting QC checks") - for item in checks: - qc_file = item["qc_result_file"] - logger.info(f"{qc_file=}") +# def run_qc_checks(checks: list) -> None: +# if checks: +# logger.info("starting QC checks") +# for item in checks: +# qc_file = item["qc_result_file"] +# logger.info(f"{qc_file=}") +# +# with open(qc_file) as fp: +# qc_data = json.load(fp) +# +# for qc_expression in item["stop_early_if"]: +# run_qc_check(qc_data, qc_expression) +# +# logger.info("QC checks finished") +# else: +# logger.info("no QC checks requested") - with open(qc_file) as fp: - qc_data = json.load(fp) - for qc_expression in item["stop_early_if"]: - run_qc_check(qc_data, qc_expression) - - logger.info("QC checks finished") - else: - logger.info("no QC checks requested") +# def run_qc_check(qc_data: dict, qc_expression: str) -> None: +# result = eval(qc_expression, globals(), qc_data) +# if result: +# logger.warning(f"failed QC check: {qc_expression}; aborting workflow execution") +# abort_execution(qc_expression) +# else: +# logger.info(f"passed QC check: {qc_expression}") -def run_qc_check(qc_data: dict, qc_expression: str) -> None: - result = eval(qc_expression, globals(), qc_data) - if result: - logger.warning(f"failed QC check: {qc_expression}; aborting") - abort_execution(qc_expression) - else: - logger.info(f"passed QC check: {qc_expression}") - - -def abort_execution(qc_expression: str) -> None: +def abort_execution(failed_expressions: list) -> None: region = os.environ["AWS_DEFAULT_REGION"] acct = os.environ["AWS_ACCOUNT_ID"] wf_name = os.environ["BC_WORKFLOW_NAME"] @@ -42,9 +43,45 @@ def abort_execution(qc_expression: str) -> None: step_name = os.environ["BC_STEP_NAME"] execution_arn = f"arn:aws:states:{region}:{acct}:execution:{wf_name}:{exec_id}" + cause = "\n".join(["failed QC conditions:"] + failed_expressions) + sfn = boto3.client("stepfunctions") sfn.stop_execution( executionArn=execution_arn, error=f"Job {exec_id} failed QC check at step {step_name}", - cause=f"failed condition: {qc_expression}" + cause=cause ) + + +def run_one_qc_check(qc_data: dict, qc_expression: str) -> bool: + if result := eval(qc_expression, globals(), qc_data): + logger.warning(f"failed QC check: {qc_expression}") + else: + logger.info(f"passed QC check: {qc_expression}") + return result + + +def run_all_qc_checks(checks: list) -> Generator[str, None, None]: + for item in checks: + qc_file = item["qc_result_file"] + logger.info(f"{qc_file=}") + + with open(qc_file) as fp: + qc_data = json.load(fp) + + for qc_expression in item["stop_early_if"]: + if run_one_qc_check(qc_data, qc_expression): + yld = f"{os.path.basename(qc_file)}: {qc_expression}" + yield yld + + +def do_checks(checks: list) -> None: + if checks: + logger.info("starting QC checks") + failures = list(run_all_qc_checks(checks)) + if failures: + logger.warning(f"aborting workflow execution") + abort_execution(failures) + logger.info("QC checks finished") + else: + logger.info("no QC checks requested") diff --git a/bclaw_runner/src/runner/runner_main.py b/bclaw_runner/src/runner/runner_main.py index 7b85140..623c4b2 100644 --- a/bclaw_runner/src/runner/runner_main.py +++ b/bclaw_runner/src/runner/runner_main.py @@ -29,11 +29,10 @@ from .cache import get_reference_inputs from .custom_logs import LOGGING_CONFIG from .string_subs import substitute, substitute_image_tag -from .qc_check import run_qc_checks +from .qc_check import do_checks from .repo import Repository from .tagging import tag_this_instance from .termination import spot_termination_checker -# from .version import VERSION from .workspace import workspace, write_job_data_file, run_commands @@ -101,8 +100,8 @@ def main(commands: List[str], # mark job complete on success if status == 0: - # todo: raise exception if qc fails - run_qc_checks(qc) + # todo: raise exception if qc fails? + do_checks(qc) repo.put_run_status() except Exception as e: diff --git a/bclaw_runner/src/runner/workspace.py b/bclaw_runner/src/runner/workspace.py index 21abfe6..dc0d3e8 100644 --- a/bclaw_runner/src/runner/workspace.py +++ b/bclaw_runner/src/runner/workspace.py @@ -3,7 +3,6 @@ import logging import os import shutil -import time from tempfile import mkdtemp, NamedTemporaryFile from .dind import run_child_container @@ -26,7 +25,6 @@ def workspace() -> str: logger.info("cleaning up workspace") os.chdir(orig_path) shutil.rmtree(work_path, ignore_errors=True) - time.sleep(15) logger.info("finished") diff --git a/bclaw_runner/tests/test_qc_check.py b/bclaw_runner/tests/test_qc_check.py index bd97b9f..a27154d 100644 --- a/bclaw_runner/tests/test_qc_check.py +++ b/bclaw_runner/tests/test_qc_check.py @@ -4,7 +4,7 @@ import moto import pytest -from ..src.runner.qc_check import run_qc_checks, run_qc_check, abort_execution +from ..src.runner.qc_check import abort_execution, run_one_qc_check, run_all_qc_checks, do_checks QC_DATA_1 = { "a": 1, @@ -43,54 +43,54 @@ def mock_qc_data_files(mocker, request): ret.side_effect = [qc_file1.return_value, qc_file2.return_value] -def test_run_qc_checks(mock_qc_data_files, mocker): - mock_run_qc_check = mocker.patch("bclaw_runner.src.runner.qc_check.run_qc_check") - - spec = [ - { - "qc_result_file": "fake1", - "stop_early_if": [ - "b == 1", - ], - }, - { - "qc_result_file": "fake2", - "stop_early_if": [ - "x == 99", - "y == 98", - ], - }, - ] - run_qc_checks(spec) - result = mock_run_qc_check.call_args_list - expect = [ - mocker.call(QC_DATA_1, "b == 1"), - mocker.call(QC_DATA_2, "x == 99"), - mocker.call(QC_DATA_2, "y == 98"), - ] - assert result == expect - - -def test_run_qc_checks_empty(mock_qc_data_files, mocker): - mock_run_qc_check = mocker.patch("bclaw_runner.src.runner.qc_check.run_qc_check") - run_qc_checks([]) - mock_run_qc_check.assert_not_called() - - -@pytest.mark.parametrize("expression, expect_abort", [ - ("x == 1", False), - ("x > 1", True), -]) -def test_run_qc_check(expression, expect_abort, mocker): - mock_abort_execution = mocker.patch("bclaw_runner.src.runner.qc_check.abort_execution") - - qc_data = {"x": 1} - run_qc_check(qc_data, expression) - - if expect_abort: - mock_abort_execution.assert_not_called() - else: - mock_abort_execution.assert_called() +# def test_run_qc_checks(mock_qc_data_files, mocker): +# mock_run_qc_check = mocker.patch("bclaw_runner.src.runner.qc_check.run_qc_check") +# +# spec = [ +# { +# "qc_result_file": "fake1", +# "stop_early_if": [ +# "b == 1", +# ], +# }, +# { +# "qc_result_file": "fake2", +# "stop_early_if": [ +# "x == 99", +# "y == 98", +# ], +# }, +# ] +# run_qc_checks(spec) +# result = mock_run_qc_check.call_args_list +# expect = [ +# mocker.call(QC_DATA_1, "b == 1"), +# mocker.call(QC_DATA_2, "x == 99"), +# mocker.call(QC_DATA_2, "y == 98"), +# ] +# assert result == expect + + +# def test_run_qc_checks_empty(mock_qc_data_files, mocker): +# mock_run_qc_check = mocker.patch("bclaw_runner.src.runner.qc_check.run_qc_check") +# run_qc_checks([]) +# mock_run_qc_check.assert_not_called() + + +# @pytest.mark.parametrize("expression, expect_abort", [ +# ("x == 1", False), +# ("x > 1", True), +# ]) +# def test_run_qc_check(expression, expect_abort, mocker): +# mock_abort_execution = mocker.patch("bclaw_runner.src.runner.qc_check.abort_execution") +# +# qc_data = {"x": 1} +# run_qc_check(qc_data, expression) +# +# if expect_abort: +# mock_abort_execution.assert_not_called() +# else: +# mock_abort_execution.assert_called() def test_abort_execution(mock_state_machine, monkeypatch): @@ -107,7 +107,70 @@ def test_abort_execution(mock_state_machine, monkeypatch): monkeypatch.setenv("BC_EXECUTION_ID", "fake_execution") monkeypatch.setenv("BC_STEP_NAME", "test_step") - abort_execution("expression that failed") + abort_execution(["failure1", "failure2"]) execution_desc = sfn.describe_execution(executionArn=sfn_execution["executionArn"]) assert execution_desc["status"] == "ABORTED" + + +@pytest.mark.parametrize("expression, expect", [ + ("x == 1", True), + ("x != 1", False), +]) +def test_run_one_qc_check(expression, expect): + qc_data = {"x": 1} + result = run_one_qc_check(qc_data, expression) + assert result == expect + + +@pytest.mark.parametrize("fake1_cond, fake2_cond, expect", [ + (["a>1"], ["x<99"], []), # all pass + (["a>1", "b==2"], ["y<98"], ["fake1: b==2"]), # one fail + (["b==1"], ["x==99", "y==98"], ["fake2: x==99", "fake2: y==98"]), # multi fail + (["a==1", "b==2"], ["x==99", "y==98"], ["fake1: a==1", "fake1: b==2", "fake2: x==99", "fake2: y==98"]), # all fail +]) +def test_run_all_qc_checks(fake1_cond, fake2_cond, expect, mock_qc_data_files): + spec = [ + { + "qc_result_file": "fake1", + "stop_early_if": fake1_cond, + }, + { + "qc_result_file": "fake2", + "stop_early_if": fake2_cond, + }, + ] + + result = list(run_all_qc_checks(spec)) + assert result == expect + + +@pytest.mark.parametrize("fake1_cond, fake2_cond, expect", [ + (None, None, False), # no checks + (["a>1"], ["x<99"], False), # all pass + (["a>1", "b==2"], ["y<98"], True), # one fail + (["b==1"], ["x==99", "y==98"], True), # multi fail + (["a==1", "b==2"], ["x==99", "y==98"], True), # all fail +]) +def test_do_checks(fake1_cond, fake2_cond, expect, mock_qc_data_files, mocker): + mock_abort_execution = mocker.patch("bclaw_runner.src.runner.qc_check.abort_execution") + + if fake1_cond is None: + spec = [] + else: + spec = [ + { + "qc_result_file": "fake1", + "stop_early_if": fake1_cond, + }, + { + "qc_result_file": "fake2", + "stop_early_if": fake2_cond, + }, + ] + + do_checks(spec) + if expect: + mock_abort_execution.assert_called_once() + else: + mock_abort_execution.assert_not_called() diff --git a/bclaw_runner/tests/test_runner_main.py b/bclaw_runner/tests/test_runner_main.py index 7364c17..9bb6c40 100644 --- a/bclaw_runner/tests/test_runner_main.py +++ b/bclaw_runner/tests/test_runner_main.py @@ -103,7 +103,7 @@ def test_main(monkeypatch, tmp_path, mock_bucket, mocker): orig_bucket_contents = {o.key for o in mock_bucket.objects.all()} - mock_run_qc_checks = mocker.patch("bclaw_runner.src.runner.runner_main.run_qc_checks") + mock_do_checks = mocker.patch("bclaw_runner.src.runner.runner_main.do_checks") response = main(image="fake_image:${job.img_tag}", commands=commands, @@ -172,7 +172,7 @@ def test_main(monkeypatch, tmp_path, mock_bucket, mocker): contents = next(fp) assert contents == "reference" - mock_run_qc_checks.assert_called_once_with(qc) + mock_do_checks.assert_called_once_with(qc) def test_main_fail_before_commands(monkeypatch, tmp_path, mock_bucket):