Skip to content

Commit

Permalink
Propagate WorkflowsService instance from WorkflowTemplate to the crea…
Browse files Browse the repository at this point in the history
…ted Workflow (#1066)

**Pull Request Checklist**
- [ ] Fixes #<!--issue number goes here-->
- [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 <[email protected]>
  • Loading branch information
AchilleAsh authored May 14, 2024
1 parent 345e687 commit 01235cf
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
1 change: 1 addition & 0 deletions src/hera/workflows/workflow_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 12 additions & 6 deletions tests/test_unit/test_workflow_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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():
Expand Down

0 comments on commit 01235cf

Please sign in to comment.