Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workflow dependencies graph visualisation #33

Merged
merged 1 commit into from
Mar 1, 2022

Conversation

itayL1
Copy link
Contributor

@itayL1 itayL1 commented Feb 23, 2022

add support for the visualization of the workflow dependencies graph of nested workflow templates (in a new isolated package).

example usage:
for this template (from the test):
image

this graph was generated:
image

@itayL1 itayL1 force-pushed the add-workflow-graph-visualization branch 30 times, most recently from dd9b2e6 to 51db884 Compare February 28, 2022 15:20
return self._graph

def add_template_subgraph(self, dsl_wf_template: WorkflowTemplate, parent_node_id: Optional[str]):
wf_template = dsl_wf_template.to_model(embed_workflow_templates=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mzedayda something weird happened here, when I changed to embed_workflow_templates=True, the tests still passed (the graph was built properly). does it make sense? do we still use templateRef s when embed_workflow_templates=True?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my mistake, it doesn't work when embed_workflow_templates=True, as it should

@itayL1 itayL1 force-pushed the add-workflow-graph-visualization branch 3 times, most recently from 7c6d3b5 to cf0b02c Compare March 1, 2022 07:41
@@ -8,7 +8,7 @@ isort:
isort argo_workflow_tools tests examples

test:
poetry run pytest --durations=5 tests/dsl
poetry run pytest --durations=5 tests/argo_workflow_tools/dsl/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only dsl?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as it was before. also, the tests in tests/argo_workflow_tools/client don't work.

def add_template_subgraph(self, deps_graph: nx.DiGraph, dsl_wf_template: WorkflowTemplate,
parent_node_id: Optional[str]):
# use embed_workflow_templates=False so the workflow model will use workflowRef when using another
# workflow template. this is how this implementation identifies the decencies in the graph.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type: dependencies

@@ -0,0 +1,21 @@
[tool.poetry]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda weird this is a separate package in the same repo no?

Are we sure we don't want to open a new repo for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's a bit weird, but I don't think that it is worth another repo, which will be less convenient. I was actually surprised that poetry doesn't support multiple packages under the same project (here is a thread on it). WDYT?

@mzedayda
Copy link
Contributor

mzedayda commented Mar 1, 2022

In the current implementation, the graph only shows parent-child dependencies, is that what we want? wouldn't it be more informative to print the actual dag?

@itayL1
Copy link
Contributor Author

itayL1 commented Mar 1, 2022

@mzedayda Yes what I want is the high-level dependencies graph between the workflows. this is very useful at least in our project, where we use nest many workflows in the complicated release flows. also, plotting the real dag is very complex, there are a lot of edge cases that are resolved by argo at runtime (loops, condition etc.).

…of nested workflow templates (in a new isolated package)
@itayL1 itayL1 force-pushed the add-workflow-graph-visualization branch from cf0b02c to eee6648 Compare March 1, 2022 09:52
@itayL1 itayL1 merged commit adacb18 into main Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants