From 2031788821916e5d8e984ddc87bc747681cfb0b0 Mon Sep 17 00:00:00 2001 From: Luis Medel Date: Mon, 18 Nov 2024 01:30:48 +0100 Subject: [PATCH] Fix too many things at once... --- .bluish/bluish.yaml | 2 +- src/bluish/actions/base.py | 12 +-- src/bluish/actions/core.py | 2 +- src/bluish/actions/docker.py | 3 +- src/bluish/contexts/__init__.py | 10 +-- src/bluish/contexts/job.py | 6 +- src/bluish/expressions.py | 12 +-- src/bluish/logging.py | 16 ++-- .../{redacted_string.py => safe_string.py} | 6 +- src/bluish/schemas.py | 82 +++++++++++++------ src/bluish/utils.py | 12 +-- test/test_core.py | 28 +++---- test/test_schemas.py | 10 +-- test/utils.py | 3 +- 14 files changed, 115 insertions(+), 89 deletions(-) rename src/bluish/{redacted_string.py => safe_string.py} (79%) diff --git a/.bluish/bluish.yaml b/.bluish/bluish.yaml index a0f7c27..551abc2 100644 --- a/.bluish/bluish.yaml +++ b/.bluish/bluish.yaml @@ -61,7 +61,7 @@ jobs: - name: Build project run: | - python -m pip install --upgrade build + python -m pip install --upgrade pip build python -m build - name: Prepare credentials diff --git a/src/bluish/actions/base.py b/src/bluish/actions/base.py index d2281b6..60b88cc 100644 --- a/src/bluish/actions/base.py +++ b/src/bluish/actions/base.py @@ -4,17 +4,7 @@ import bluish.process from bluish.contexts import Definition from bluish.logging import debug -from bluish.schemas import KV, validate_schema - - -class RequiredInputError(Exception): - def __init__(self, param: str): - super().__init__(f"Missing required input parameter: {param}") - - -class RequiredAttributeError(Exception): - def __init__(self, param: str): - super().__init__(f"Missing required attribute: {param}") +from bluish.schemas import validate_schema def _key_exists(key: str, attrs: Definition) -> bool: diff --git a/src/bluish/actions/core.py b/src/bluish/actions/core.py index 9a97692..61d1046 100644 --- a/src/bluish/actions/core.py +++ b/src/bluish/actions/core.py @@ -15,7 +15,7 @@ class RunCommand(bluish.actions.base.Action): SCHEMA = { "type": dict, "properties": { - "run": [str, None], + "run": str, "shell": [str, None], }, } diff --git a/src/bluish/actions/docker.py b/src/bluish/actions/docker.py index c5d3a3b..be8ba9b 100644 --- a/src/bluish/actions/docker.py +++ b/src/bluish/actions/docker.py @@ -5,6 +5,7 @@ import bluish.contexts.step import bluish.process from bluish.logging import debug, error, info, warning +from bluish.schemas import STR_LIST from bluish.utils import decorate_for_log @@ -106,7 +107,7 @@ class Build(bluish.actions.base.Action): "type": dict, "properties": { "dockerfile": [str, None], - "tags": [str, list[str]], + "tags": [str, STR_LIST], "context": [str, None], }, } diff --git a/src/bluish/contexts/__init__.py b/src/bluish/contexts/__init__.py index 008416d..3e462c5 100644 --- a/src/bluish/contexts/__init__.py +++ b/src/bluish/contexts/__init__.py @@ -6,7 +6,7 @@ import bluish.core import bluish.process from bluish.logging import info -from bluish.redacted_string import RedactedString +from bluish.safe_string import SafeString from bluish.schemas import ( JOB_SCHEMA, STEP_SCHEMA, @@ -274,7 +274,7 @@ def prepare_value(value: Any) -> Any: wf = cast(bluish.contexts.workflow.WorkflowContext, _workflow(ctx)) if varname in wf.secrets: return prepare_value( - RedactedString(cast(str, wf.secrets[varname]), "********") + SafeString(cast(str, wf.secrets[varname]), "********") ) elif root == "jobs": wf = cast(bluish.contexts.workflow.WorkflowContext, _workflow(ctx)) @@ -288,7 +288,7 @@ def prepare_value(value: Any) -> Any: elif root == "steps": job = cast(bluish.contexts.job.JobContext, _job(ctx)) step_id, varname = varname.split(".", maxsplit=1) - step = job.steps.get(step_id) + step = next((step for step in job.steps if step.id == step_id), None) if not step: raise ValueError(f"Step {step_id} not found") return _try_get_value(step, varname, raw) @@ -302,7 +302,7 @@ def prepare_value(value: Any) -> Any: node = _step_or_job(ctx) if varname in node.inputs: if varname in node.sensitive_inputs: - return prepare_value(RedactedString(node.inputs[varname], "********")) + return prepare_value(SafeString(node.inputs[varname], "********")) else: return prepare_value(node.inputs[varname]) elif root == "outputs": @@ -346,7 +346,7 @@ def _try_set_value(ctx: "ContextNode", name: str, value: str) -> bool: elif root == "steps": job = cast(bluish.contexts.job.JobContext, _job(ctx)) step_id, varname = varname.split(".", maxsplit=1) - step = job.steps.get(step_id) + step = next((step for step in job.steps if step.id == step_id), None) if not step: raise ValueError(f"Step {step_id} not found") return _try_set_value(step, varname, value) diff --git a/src/bluish/contexts/job.py b/src/bluish/contexts/job.py index 3f8a631..6b7393d 100644 --- a/src/bluish/contexts/job.py +++ b/src/bluish/contexts/job.py @@ -41,12 +41,12 @@ def __init__( **self.attrs.var, } - self.steps: dict[str, bluish.contexts.step.StepContext] = {} + self.steps: list[bluish.contexts.step.StepContext] = [] for i, step_dict in enumerate(self.attrs.steps): step_def = contexts.StepDefinition(step_dict) step_def.ensure_property("id", f"step_{i+1}") step = bluish.contexts.step.StepContext(self, step_def) - self.steps[step_id] = step + self.steps.append(step) def dispatch(self) -> bluish.process.ProcessResult | None: self.status = bluish.core.ExecutionStatus.RUNNING @@ -71,7 +71,7 @@ def dispatch(self) -> bluish.process.ProcessResult | None: info("Job skipped") return None - for step in self.steps.values(): + for step in self.steps: result = step.dispatch() if not result: continue diff --git a/src/bluish/expressions.py b/src/bluish/expressions.py index 23fd523..ace9d50 100644 --- a/src/bluish/expressions.py +++ b/src/bluish/expressions.py @@ -4,7 +4,7 @@ import lark import bluish.contexts -from bluish.redacted_string import RedactedString +from bluish.safe_string import SafeString SENSITIVE_LITERALS = ("password", "secret", "token") @@ -91,12 +91,12 @@ def concat(a: Any, b: Any) -> Any: if b is None: return a - result = RedactedString(str(a) + str(b)) + result = SafeString(str(a) + str(b)) result.redacted_value = ( - a.redacted_value if isinstance(a, RedactedString) else str(a) + a.redacted_value if isinstance(a, SafeString) else str(a) ) result.redacted_value += ( - b.redacted_value if isinstance(b, RedactedString) else str(b) + b.redacted_value if isinstance(b, SafeString) else str(b) ) return result @@ -111,8 +111,8 @@ def number(self, value: str) -> int | float: return to_number(value) def str(self, value: str) -> str: - if isinstance(value, RedactedString): - return RedactedString(value[1:-1], value.redacted_value[1:-1]) + if isinstance(value, SafeString): + return SafeString(value[1:-1], value.redacted_value[1:-1]) elif isinstance(value, str): return value[1:-1] else: diff --git a/src/bluish/logging.py b/src/bluish/logging.py index f5fb391..ac16cf9 100644 --- a/src/bluish/logging.py +++ b/src/bluish/logging.py @@ -7,39 +7,39 @@ from logging import warning as logging_warning from typing import Any -from bluish.redacted_string import RedactedString +from bluish.safe_string import SafeString def info(message: str, *args: Any, **kwargs: Any) -> None: - msg = message.redacted_value if isinstance(message, RedactedString) else message + msg = message.redacted_value if isinstance(message, SafeString) else message logging_info(msg, *args, **kwargs) def error(message: str, *args: Any, **kwargs: Any) -> None: - msg = message.redacted_value if isinstance(message, RedactedString) else message + msg = message.redacted_value if isinstance(message, SafeString) else message logging_error(msg, *args, **kwargs) def warning(message: str, *args: Any, **kwargs: Any) -> None: - msg = message.redacted_value if isinstance(message, RedactedString) else message + msg = message.redacted_value if isinstance(message, SafeString) else message logging_warning(msg, *args, **kwargs) def debug(message: str, *args: Any, **kwargs: Any) -> None: - msg = message.redacted_value if isinstance(message, RedactedString) else message + msg = message.redacted_value if isinstance(message, SafeString) else message logging_debug(msg, *args, **kwargs) def critical(message: str, *args: Any, **kwargs: Any) -> None: - msg = message.redacted_value if isinstance(message, RedactedString) else message + msg = message.redacted_value if isinstance(message, SafeString) else message logging_critical(msg, *args, **kwargs) def exception(message: str, *args: Any, **kwargs: Any) -> None: - msg = message.redacted_value if isinstance(message, RedactedString) else message + msg = message.redacted_value if isinstance(message, SafeString) else message logging_exception(msg, *args, **kwargs) def log(level: int, message: str, *args: Any, **kwargs: Any) -> None: - msg = message.redacted_value if isinstance(message, RedactedString) else message + msg = message.redacted_value if isinstance(message, SafeString) else message logging_log(level, msg, *args, **kwargs) diff --git a/src/bluish/redacted_string.py b/src/bluish/safe_string.py similarity index 79% rename from src/bluish/redacted_string.py rename to src/bluish/safe_string.py index fc60834..0feaecf 100644 --- a/src/bluish/redacted_string.py +++ b/src/bluish/safe_string.py @@ -1,11 +1,11 @@ from typing import cast -class RedactedString(str): +class SafeString(str): """A string with two values: one for logging and one for the actual value.""" - def __new__(cls, value: str, redacted_value: str | None = None) -> "RedactedString": - result = cast(RedactedString, super().__new__(cls, value)) + def __new__(cls, value: str, redacted_value: str | None = None) -> "SafeString": + result = cast(SafeString, super().__new__(cls, value)) result.redacted_value = redacted_value or value return result diff --git a/src/bluish/schemas.py b/src/bluish/schemas.py index 6319823..49c7044 100644 --- a/src/bluish/schemas.py +++ b/src/bluish/schemas.py @@ -1,9 +1,33 @@ -from typing import Any, Union +from typing import Any, Iterable, Union + +from bluish.safe_string import SafeString + + +class InvalidTypeError(Exception): + def __init__(self, value: Any, types: str): + super().__init__(f"{value} is not any of the allowed types: {types}") + + +class RequiredAttributeError(Exception): + def __init__(self, param: str): + super().__init__(f"Missing required attribute: {param}") + + +class UnexpectedAttributesError(Exception): + def __init__(self, attrs: Iterable[str]): + super().__init__(f"Unexpected attributes: {attrs}") + KV = { "type": dict, - "key_schema": {"type": str}, - "value_schema": {"type": str}, + "key_schema": str, + "value_schema": [str, bool, int, float, None], +} + +STR_KV = { + "type": dict, + "key_schema": str, + "value_schema": str, } STR_LIST = { @@ -17,7 +41,7 @@ "name": [str, None], "env": [KV, None], "var": [KV, None], - "secrets": [KV, None], + "secrets": [STR_KV, None], "secrets_file": [str, None], "env_file": [str, None], "uses": [str, None], @@ -36,7 +60,7 @@ "name": [str, None], "env": [KV, None], "var": [KV, None], - "secrets": [KV, None], + "secrets": [STR_KV, None], "secrets_file": [str, None], "env_file": [str, None], "runs_on": [str, None], @@ -56,7 +80,7 @@ "name": [str, None], "env": [KV, None], "var": [KV, None], - "secrets": [KV, None], + "secrets": [STR_KV, None], "secrets_file": [str, None], "env_file": [str, None], "runs_on": [str, None], @@ -84,21 +108,32 @@ def _get_type_repr(t: type_def | None) -> str: return f"{t}" -def _find_type(value: Any, t: type_def | None) -> dict | type | None: - if value is None or t is None: +def _find_type(value: Any, _def: type_def | None) -> dict | type | None: + def get_origin(t): + if t is None: + return None + if isinstance(t, SafeString): + return str + return get_origin(t.__origin__) if "__origin__" in t.__dict__ else t + + if value is None or _def is None: return None - elif isinstance(t, list): - return next((tt for tt in t if _find_type(value, tt)), None) # type: ignore - elif isinstance(t, dict): - if "type" not in t: + elif isinstance(_def, list): + for tt in _def: + if _find_type(value, tt): + return tt + return None + elif isinstance(_def, dict): + _t = _def.get("type") + if _t is None: return None - if t["type"] is Any: - return t - return t if type(value) is t["type"] else None + if _t is Any: + return _def + return _def if get_origin(type(value)) is get_origin(_t) else None else: - if t is Any: - return t # type: ignore - return t if type(value) is t else None + if _def is Any: + return _def # type: ignore + return _def if get_origin(type(value)) is get_origin(_def) else None def _is_required(t: type_def | None) -> bool: @@ -138,12 +173,13 @@ def validate_schema( ... ValueError: 42 is not any of the allowed types: """ + + if data is None and not _is_required(schema): + return type_def = _find_type(data, schema) if type_def is None: - raise ValueError( - f"{data} is not any of the allowed types: {_get_type_repr(schema)}" - ) + raise InvalidTypeError(data, _get_type_repr(schema)) if isinstance(type_def, type) or type_def is Any: return @@ -164,13 +200,13 @@ def validate_schema( if data.get(prop) is None: if not _is_required(tdef): continue - raise ValueError(f"Missing required key: {prop}") + raise RequiredAttributeError(f"Missing required key: {prop}") validate_schema(tdef, data[prop]) if reject_extra_keys and "properties" in type_def: extra_keys = set(data.keys()) - set(type_def["properties"].keys()) if extra_keys: - raise ValueError(f"Extra keys: {extra_keys}") + raise UnexpectedAttributesError(extra_keys) elif type_def["type"] == list: assert isinstance(data, list) item_schema = type_def["item_schema"] diff --git a/src/bluish/utils.py b/src/bluish/utils.py index d2c578b..fc559e5 100644 --- a/src/bluish/utils.py +++ b/src/bluish/utils.py @@ -1,18 +1,18 @@ # The dreaded "utils" module, where lazy programmers put all the miscellaneous functions. -from bluish.redacted_string import RedactedString +from bluish.safe_string import SafeString -def safe_string(value: str | RedactedString) -> str: +def safe_string(value: str | SafeString) -> str: """Returns the string value of a RedactedString.""" - if isinstance(value, RedactedString): + if isinstance(value, SafeString): return value.redacted_value else: return value -def decorate_for_log(value: str | RedactedString, decoration: str = " ") -> str: +def decorate_for_log(value: str | SafeString, decoration: str = " ") -> str: """Decorates a multiline string for pretty logging.""" def decorate(value: str, decoration: str) -> str: @@ -25,8 +25,8 @@ def decorate(value: str, decoration: str) -> str: lines = value.rstrip().splitlines(keepends=True) return "\n" + "".join(f"{decoration}{line}" for line in lines) - if isinstance(value, RedactedString): - result = RedactedString(value) + if isinstance(value, SafeString): + result = SafeString(value) result.redacted_value = decorate(value.redacted_value, decoration) return result else: diff --git a/test/test_core.py b/test/test_core.py index 381c406..8c67344 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -4,7 +4,7 @@ from test.utils import create_workflow import pytest -from bluish.actions.base import RequiredAttributeError, RequiredInputError +from bluish.schemas import RequiredAttributeError from bluish.contexts import CircularDependencyError from bluish.core import ( ExecutionStatus, @@ -353,7 +353,7 @@ def test_mandatory_inputs() -> None: try: _ = wf.dispatch() raise AssertionError("Mandatory input not detected") - except RequiredInputError: + except RequiredAttributeError: pass @@ -392,7 +392,7 @@ def test_expansion() -> None: def test_expansion_with_ambiguity() -> None: wf = create_workflow(""" var: - test_result: + test_result: stdout: "No" jobs: @@ -467,11 +467,9 @@ def test_values() -> None: assert wf.get_value("var.VALUE") == 1 assert wf.get_value("workflow.var.VALUE") == 1 assert wf.get_value("workflow.jobs.test_job.var.VALUE") == 2 - assert wf.jobs["test_job"].get_value("var.VALUE") == 2 - assert wf.jobs["test_job"].steps["step-1"].get_value("var.VALUE") == 2 - assert wf.jobs["test_job"].steps["step-1"].get_value("job.var.VALUE") == 2 - assert wf.jobs["test_job"].get_value("var.VALUE") == 2 - assert wf.jobs["test_job"].get_value("job.var.VALUE") == 2 + assert wf.get_value("jobs.test_job.var.VALUE") == 2 + assert wf.get_value("jobs.test_job.steps.step-1.var.VALUE") == 2 + assert wf.get_value("jobs.test_job.var.VALUE") == 2 def test_expressions() -> None: @@ -547,14 +545,14 @@ def test_set() -> None: set: workflow.var.VALUE: 2 job.var.COOL_YEAR: 1980 - jobs.test_job.var.VOUTPUT: ${{ .stdout }} + jobs.test_job.var.OUTPUT: ${{ .stdout }} """) _ = wf.dispatch() assert wf.get_value("var.VALUE") == 2 assert wf.get_value("jobs.test_job.var.COOL_YEAR") == 1980 - assert wf.jobs["test_job"].steps["step-1"].result.stdout == "VALUE == 42" - assert wf.jobs["test_job"].var["VOUTPUT"] == "VALUE == 42" + assert wf.get_value("jobs.test_job.steps.step-1.stdout") == "VALUE == 42" + assert wf.get_value("jobs.test_job.var.OUTPUT") == "VALUE == 42" def test_env_overriding() -> None: @@ -585,9 +583,9 @@ def test_env_overriding() -> None: """) _ = wf.dispatch() - assert wf.jobs["test_job"].steps["step-1"].result.stdout == "Hello World! :-DDDD" - assert wf.jobs["test_job"].steps["step-2"].result.stdout == "Hello World! xD" - assert wf.jobs["test_job_2"].steps["step-1"].result.stdout == "Hello World! :-(" + assert wf.get_value("jobs.test_job.steps.step-1.stdout") == "Hello World! :-DDDD" + assert wf.get_value("jobs.test_job.steps.step-2.stdout") == "Hello World! xD" + assert wf.get_value("jobs.test_job_2.steps.step-1.stdout") == "Hello World! :-(" def test_expand_template(temp_file: FileIO) -> None: @@ -815,5 +813,5 @@ def test_docker_file_download_failed(temp_file: FileIO) -> None: _ = wf.dispatch() assert wf.jobs["test_job"].failed - assert wf.jobs["test_job"].steps["step_1"].failed + assert wf.jobs["test_job"].steps[0].failed assert wf.get_value("jobs.test_job.steps.step_2.stdout") == "" diff --git a/test/test_schemas.py b/test/test_schemas.py index d179aad..e7c9bce 100644 --- a/test/test_schemas.py +++ b/test/test_schemas.py @@ -1,7 +1,7 @@ from typing import Any -from bluish.schemas import KV, validate_schema +from bluish.schemas import KV, InvalidTypeError, RequiredAttributeError, validate_schema def test_happy() -> None: @@ -39,8 +39,8 @@ def test_missing_key() -> None: try: validate_schema(schema, data) raise AssertionError("Should have raised an exception") - except ValueError as e: - assert str(e) == "Missing required key: env" + except RequiredAttributeError: + pass def test_optional_key() -> None: @@ -111,5 +111,5 @@ def test_lists_ko() -> None: try: validate_schema(schema, data) raise AssertionError("Should have raised an exception") - except ValueError as e: - assert str(e) == "1 is not any of the allowed types: " + except InvalidTypeError: + pass diff --git a/test/utils.py b/test/utils.py index c98583e..392dce1 100644 --- a/test/utils.py +++ b/test/utils.py @@ -1,8 +1,9 @@ +from bluish.contexts import WorkflowDefinition import bluish.contexts.workflow import yaml def create_workflow(yaml_definition: str) -> bluish.contexts.workflow.WorkflowContext: - definition = yaml.safe_load(yaml_definition) + definition = WorkflowDefinition(yaml.safe_load(yaml_definition)) return bluish.contexts.workflow.WorkflowContext(definition)