From 01235cfa1502615ec05d47465c4a7cf3a09d6894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9=20Perez?= Date: Tue, 14 May 2024 06:48:32 +0200 Subject: [PATCH] Propagate WorkflowsService instance from WorkflowTemplate to the created Workflow (#1066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Pull Request Checklist** - [ ] Fixes # - [x] Tests added - [ ] Documentation/examples added - [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR title **Description of PR** Currently, when creating a workflow from a workflow template, it implicitly relies on the global config for the WorkflowsService, as it is not copying the service from the template. Thus, the simple example in the doc would fail, useless the Argo host is defined in the global config. This PR binds the WorkflowsService instance of the WorkflowTemplate to the created Workflow to propagate non-global configuration, and changes 2 tests to make sure the config is actually propagated. Thanks! Signed-off-by: Timothé Perez --- src/hera/workflows/workflow_template.py | 1 + tests/test_unit/test_workflow_template.py | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/hera/workflows/workflow_template.py b/src/hera/workflows/workflow_template.py index 651d434c7..3ed063daf 100644 --- a/src/hera/workflows/workflow_template.py +++ b/src/hera/workflows/workflow_template.py @@ -153,6 +153,7 @@ def from_file(cls, yaml_file: Union[Path, str]) -> ModelMapperMixin: def _get_as_workflow(self, generate_name: Optional[str]) -> Workflow: workflow = cast(Workflow, Workflow.from_dict(self.to_dict())) workflow.kind = "Workflow" + workflow.workflows_service = self.workflows_service # bind this workflow to the same service if generate_name is not None: workflow.generate_name = generate_name diff --git a/tests/test_unit/test_workflow_template.py b/tests/test_unit/test_workflow_template.py index 47b496de1..03b4b3a23 100644 --- a/tests/test_unit/test_workflow_template.py +++ b/tests/test_unit/test_workflow_template.py @@ -48,11 +48,11 @@ def test_workflow_template_create(global_config_fixture): def test_workflow_template_create_as_workflow(): - with patch.object(WorkflowsService, "create_workflow", return_value=MagicMock()) as create_workflow: + with patch.object(WorkflowsService, "create_workflow", return_value=MagicMock(), autospec=True) as create_workflow: # We have to patch the function at the class level because create_as_workflow copies the workflows service # from the WorkflowTemplate to the *separate* Workflow object. - ws = WorkflowsService(namespace="my-namespace") + ws = WorkflowsService(namespace="my-namespace", host="https://localhost:2746") # Note we set the name to None here, otherwise the workflow will take the name from the returned object create_workflow.return_value.metadata.name = None @@ -75,17 +75,22 @@ def test_workflow_template_create_as_workflow(): wt.create_as_workflow() # THEN - wt.workflows_service.create_workflow.assert_called_once_with( - WorkflowCreateRequest(workflow=expected_workflow.build()), - namespace="my-namespace", - ) + create_workflow.assert_called_once() + args, kwargs = create_workflow.call_args + assert kwargs == {"namespace": "my-namespace"} + wf_ws, req = args + assert req == WorkflowCreateRequest(workflow=expected_workflow.build()) + assert wf_ws == ws def test_workflow_template_get_as_workflow(): + ws = WorkflowsService(host="https://localhost:2746") + # GIVEN with WorkflowTemplate( name="my-wt", namespace="my-namespace", + workflows_service=ws, ) as wt: pass @@ -97,6 +102,7 @@ def test_workflow_template_get_as_workflow(): assert workflow.kind == "Workflow" assert workflow.name is None assert workflow.generate_name == "my-wt" + assert workflow.workflows_service == ws def test_workflow_template_get_as_workflow_truncator():