From 9565fa4640a0ab675f9b525efed6800ee67d9c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Mart=C3=ADn?= Date: Wed, 9 Oct 2019 06:45:57 +0000 Subject: [PATCH] Workspace initial support --- jeeves/core/actions/base.py | 5 +++ jeeves/core/actions/shell.py | 12 +++++- jeeves/core/actions/stub.py | 6 +-- jeeves/core/executor.py | 14 ++++++- jeeves/core/objects.py | 25 +++++++++++++ jeeves/core/tests/conftest.py | 16 ++++++++ jeeves/core/tests/tasks/test_shell.py | 37 ++++++++++++------- .../tests/{test_tasks.py => test_actions.py} | 0 jeeves/core/tests/test_executor.py | 24 +++++++++++- 9 files changed, 117 insertions(+), 22 deletions(-) rename jeeves/core/tests/{test_tasks.py => test_actions.py} (100%) diff --git a/jeeves/core/actions/base.py b/jeeves/core/actions/base.py index 940bc14..0fca8c2 100644 --- a/jeeves/core/actions/base.py +++ b/jeeves/core/actions/base.py @@ -1,4 +1,5 @@ import logging +from abc import abstractmethod import pydantic @@ -12,3 +13,7 @@ class Action: def __init__(self, parameters=None): self.logger = logging.getLogger(self.__class__.__name__) self.parameters = self.Parameters(**(parameters or {})) + + @abstractmethod + def execute(self, workspace, **kwargs): + pass diff --git a/jeeves/core/actions/shell.py b/jeeves/core/actions/shell.py index 379ad9a..40f4bd7 100644 --- a/jeeves/core/actions/shell.py +++ b/jeeves/core/actions/shell.py @@ -18,6 +18,9 @@ class ScriptAction(Action): If no shebang is provided, a default of ``#!/bin/bash -e`` will be used, if the provided shebang interpreter is not found on the system the action will fail. + The working directory for the process is the workspace path by default. This path + is also exposed as the ``WORKSPACE_PATH`` environment variable. + .. automethod:: _get_script """ @@ -47,7 +50,8 @@ class ScriptAction(Action): return f"{self.DEFAULT_SHEBANG}{os.linesep}{self.parameters.script}" return self.parameters.script - def execute(self): + def execute(self, **kwargs): + workspace = kwargs.get("workspace") script = self._get_script() # Write the script to a temporary file @@ -59,7 +63,11 @@ class ScriptAction(Action): os.chmod(script_file.name, mode=0o500) process = subprocess.run( - script_file.name, stdout=subprocess.PIPE, stderr=subprocess.STDOUT + script_file.name, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + cwd=workspace.path, + env={"WORKSPACE_PATH": workspace.path}, ) os.unlink(script_file.name) diff --git a/jeeves/core/actions/stub.py b/jeeves/core/actions/stub.py index 5783a61..677ba3b 100644 --- a/jeeves/core/actions/stub.py +++ b/jeeves/core/actions/stub.py @@ -10,21 +10,21 @@ from .base import Action class StubSuccessAction(Action): id = "stub/success" - def execute(self, message=None): + def execute(self, **kwargs): return Result(success=True) class StubNonSuccessAction(Action): id = "stub/non-success" - def execute(self): + def execute(self, **kwargs): return Result(output="error!", success=False) class StubUncaughtExceptionAction(Action): id = "stub/uncaught-exception" - def execute(self): + def execute(self, **kwargs): raise Exception("Oh god...") diff --git a/jeeves/core/executor.py b/jeeves/core/executor.py index 131b6f3..3084fde 100644 --- a/jeeves/core/executor.py +++ b/jeeves/core/executor.py @@ -20,7 +20,7 @@ class Executor: def execute_step(self, step: ExecutionStep): try: - step.result = step.task.action.execute() + step.result = step.task.action.execute(workspace=self._execution.workspace) except Exception as error: # Catch unhandled exceptions, mark the result as unsuccessful # and append the error as output. @@ -39,6 +39,16 @@ class Executor: return step.result def start(self): - for step in self._execution.steps: + """ + Executes (sync) all the actions for the provided flow. + """ + for step in self.steps: self.execute_step(step) self._execution.success = step.result.success + self.post_execution() + + def post_execution(self): + """ + Cleanup after a flow have been executed. + """ + self._execution.workspace.destroy() diff --git a/jeeves/core/objects.py b/jeeves/core/objects.py index 15e20a6..11c996b 100644 --- a/jeeves/core/objects.py +++ b/jeeves/core/objects.py @@ -1,4 +1,7 @@ +import shutil +import tempfile from typing import Any, Dict, List, Text, Optional +from pathlib import Path from dataclasses import field import pydantic @@ -38,11 +41,33 @@ class ExecutionStep(BaseObject): result: Optional[Result] = None +class Workspace(BaseObject): + path: Path = None # type: ignore + + @pydantic.validator("path", pre=True, always=True) + def path_default(cls, v): # pylint: disable=no-self-argument + """ + Ensures that a Workspace always have a path set up. + """ + return v or tempfile.mkdtemp(prefix="jeeves_") + + def destroy(self): + """ + Removes the workspace path from the filesystem + """ + shutil.rmtree(self.path) + + class Execution(BaseObject): flow: Flow steps: List[ExecutionStep] + workspace: Workspace = None # type: ignore success: bool = False + @pydantic.validator("workspace", pre=True, always=True) + def workspace_default(cls, v): # pylint: disable=no-self-argument + return v or Workspace() + @property def output(self): for step in self.steps: diff --git a/jeeves/core/tests/conftest.py b/jeeves/core/tests/conftest.py index 18e60c9..cd92492 100644 --- a/jeeves/core/tests/conftest.py +++ b/jeeves/core/tests/conftest.py @@ -1,8 +1,24 @@ +import os + import pytest +from jeeves.core.objects import Workspace from jeeves.core.registry import ActionRegistry @pytest.fixture(scope="session", autouse=True) def autoregister_actions(): ActionRegistry.autodiscover() + + +@pytest.fixture +def workspace_obj(): + """ + Fixture that returns a :any:`core.objects.Workspace` object and then ensures + the path it's deleted. + + Used for action tests. + """ + workspace = Workspace() + yield workspace + os.rmdir(workspace.path) diff --git a/jeeves/core/tests/tasks/test_shell.py b/jeeves/core/tests/tasks/test_shell.py index d0271ad..1199165 100644 --- a/jeeves/core/tests/tasks/test_shell.py +++ b/jeeves/core/tests/tasks/test_shell.py @@ -19,35 +19,35 @@ def get_completed_process(returncode=0, stdout=b"", **kwargs): @mock.patch("subprocess.run", mock.MagicMock(return_value=get_completed_process())) -def test_script_bash_task_ok(): +def test_script_bash_task_ok(workspace_obj): task = Task.parse_obj(MOCK_DEFINITION).action - result = task.execute() + result = task.execute(workspace=workspace_obj) assert result.success @mock.patch( "subprocess.run", mock.MagicMock(return_value=get_completed_process(returncode=1)) ) -def test_script_bash_task_ko(): +def test_script_bash_task_ko(workspace_obj): task = Task.parse_obj(MOCK_DEFINITION).action - result = task.execute() + result = task.execute(workspace=workspace_obj) assert not result.success @mock.patch("subprocess.run", mock.MagicMock(return_value=get_completed_process())) -def test_script_no_shebang_defaults_to_bash_ok(): +def test_script_no_shebang_defaults_to_bash_ok(workspace_obj): definition = MOCK_DEFINITION.copy() definition["parameters"]["script"] = definition["parameters"]["script"].strip( "#!/bin/bash" ) task = Task.parse_obj(definition).action assert task._get_script().startswith(task.DEFAULT_SHEBANG) - result = task.execute() + result = task.execute(workspace=workspace_obj) assert result.success @mock.patch("subprocess.run", mock.MagicMock(return_value=get_completed_process())) -def test_script_with_other_shebang_ok(): +def test_script_with_other_shebang_ok(workspace_obj): py_interpreter = sys.executable expected_output = "Hello world! (from python)" definition = MOCK_DEFINITION.copy() @@ -55,11 +55,11 @@ def test_script_with_other_shebang_ok(): definition["parameters"]["script"] = py_script task = Task.parse_obj(definition).action assert task._get_script().startswith(f"#!{py_interpreter}") - result = task.execute() + result = task.execute(workspace=workspace_obj) assert result.success -def test_script_stdout_and_stderr_is_sent_to_result_ok(): +def test_script_stdout_and_stderr_is_sent_to_result_ok(workspace_obj): """ ..warning:: This test actually calls ``subprocess.run``. @@ -78,25 +78,25 @@ def test_script_stdout_and_stderr_is_sent_to_result_ok(): definition = MOCK_DEFINITION.copy() definition["parameters"]["script"] = script task = Task.parse_obj(definition).action - result = task.execute() + result = task.execute(workspace=workspace_obj) assert "Hello" in result.output assert "World" in result.output @mock.patch("subprocess.run", mock.MagicMock(return_value=get_completed_process())) -def test_script_task_cleans_tempfile_ok(): +def test_script_task_cleans_tempfile_ok(workspace_obj): """Make sure that the script is removed from the system after execution""" task = Task.parse_obj(MOCK_DEFINITION).action temp = tempfile.NamedTemporaryFile(mode="w", delete=False) with mock.patch( "tempfile.NamedTemporaryFile", mock.MagicMock(return_value=temp) ) as mocked: - task.execute() + task.execute(workspace=workspace_obj) assert not os.path.isfile(mocked.return_value.name) @mock.patch("subprocess.run", mock.MagicMock(return_value=get_completed_process())) -def test_script_task_sets_permissions_for_owner_only_ok(): +def test_script_task_sets_permissions_for_owner_only_ok(workspace_obj): """Make sure that the script have only read and execution permissions for owner""" task = Task.parse_obj(MOCK_DEFINITION).action temp = tempfile.NamedTemporaryFile(mode="w", delete=False) @@ -104,7 +104,16 @@ def test_script_task_sets_permissions_for_owner_only_ok(): "tempfile.NamedTemporaryFile", mock.MagicMock(return_value=temp) ) as mocked: with mock.patch("os.unlink"): - task.execute() + task.execute(workspace=workspace_obj) stat = os.stat(mocked.return_value.name) assert oct(stat.st_mode).endswith("500") os.unlink(mocked.return_value.name) + + +@mock.patch("subprocess.run") +def test_script_task_appends_workspace_env_variable_ok(run_mock, workspace_obj): + """Make sure that the WORKSPACE_PATH environment variable is sent correctly """ + run_mock.return_value = get_completed_process() + task = Task.parse_obj(MOCK_DEFINITION).action + task.execute(workspace=workspace_obj) + assert run_mock.call_args[1]["env"] == {"WORKSPACE_PATH": workspace_obj.path} diff --git a/jeeves/core/tests/test_tasks.py b/jeeves/core/tests/test_actions.py similarity index 100% rename from jeeves/core/tests/test_tasks.py rename to jeeves/core/tests/test_actions.py diff --git a/jeeves/core/tests/test_executor.py b/jeeves/core/tests/test_executor.py index 6d96c4d..cefe94f 100644 --- a/jeeves/core/tests/test_executor.py +++ b/jeeves/core/tests/test_executor.py @@ -1,5 +1,8 @@ +import os.path +from unittest import mock + from jeeves.core.executor import Executor -from .factories import FlowFactory, TaskFactory +from jeeves.core.tests.factories import FlowFactory, TaskFactory def test_executor_success_task_ok(): @@ -30,3 +33,22 @@ def test_executor_uncaught_exception_in_task_ok(): assert runner._execution.steps[0].result assert runner._execution.steps[0].result.success is False assert runner._execution.success is False + + +@mock.patch("jeeves.core.actions.stub.StubSuccessAction.execute") +def test_executor_run_action_with_workpsace_ok(execute_mock): + task = TaskFactory(type="jeeves.core.actions.stub:StubSuccessAction") + flow = FlowFactory(tasks=[task]) + runner = Executor(flow) + runner.start() + assert execute_mock.called + execute_mock.assert_called_with(workspace=runner._execution.workspace) + + +def test_executor_cleans_workspace_after_ok(): + task = TaskFactory(type="jeeves.core.actions.stub:StubSuccessAction") + flow = FlowFactory(tasks=[task]) + runner = Executor(flow) + path = runner._execution.workspace.path + runner.start() + assert not os.path.isdir(path)