Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Visualize graphs on command line #75

Merged
merged 16 commits into from
Jun 18, 2021
Merged

Visualize graphs on command line #75

merged 16 commits into from
Jun 18, 2021

Conversation

kumare3
Copy link
Contributor

@kumare3 kumare3 commented May 28, 2021

Signed-off-by: Ketan Umare [email protected]

TL;DR

Uses graphviz to visualize flyte workflows on commandline. Example of branch node
nested-conditionals
coin-toss
sub-workflows

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Uses graphviz to visualize flyte workflows on commandline

@kumare3 kumare3 requested a review from pmahindrakar-oss May 28, 2021 00:17
@kumare3 kumare3 changed the title [wip]: Visualize graphs on command line Visualize graphs on command line Jun 1, 2021
@jsonporter
Copy link

Graph looks good (once tests pass)

}
if n.Metadata != nil && n.Metadata.Name != "" {
v := strings.LastIndexAny(n.Metadata.Name, ".")
gn.SetLabel(fmt.Sprintf("%s [%s]", n.Metadata.Name[v+1:], t.Template.Type))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: in the examples it looks like the task type always goes outside the bounding box for the task node - is this fixable here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, fixable probably by using a better way of rendering

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what if we do the beautification as a separate pass?

Copy link
Contributor

Choose a reason for hiding this comment

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

💯 sounds good!

@kumare3 kumare3 requested a review from yindia June 2, 2021 05:07
@yindia
Copy link
Contributor

yindia commented Jun 3, 2021

@pmahindrakar-oss @kumare3 @@EngHabu

This pr need CGO enable in gobuild that's why it's failing. Goreleaser doesn't support CGO build. I explored some solution around this problem. Community is building CGO enabled build in container, They use different images for different PLATFORM/ARC and in our case it become complicated because we are generating multiple binary and may be we will add new binaries in future
Docker images: https://github.com/elastic/golang-crossbuild

second problem is that we use goreleaser for generating install.sh that we use in our docker images for installing flytectl, That will break

Third problem is brew, Currently brew package is published by goreleaser or else we need to do this manually

Need your thoughts on this. Alternative option is to use different library for graph (https://github.com/awalterschulze/gographviz)

Issue : #1082

@kumare3
Copy link
Contributor Author

kumare3 commented Jun 3, 2021

@evalsocket this is really sad. The reason I used this library is because it brings the c-bindings with it. I.e no need of external dot library. Can we limit some go packages only for some platforms like osx

@yindia
Copy link
Contributor

yindia commented Jun 3, 2021

@kumare3 not sure. This lib look strong https://github.com/awalterschulze/gographviz. I discussed with @pmahindrakar-oss and he is exploring the capabilities of this lib.

@kumare3
Copy link
Contributor Author

kumare3 commented Jun 3, 2021

The lib will only print a dot graph - and users will have to use an online renderer

@kumare3
Copy link
Contributor Author

kumare3 commented Jun 3, 2021

I guess maybe we have to do this

@yindia
Copy link
Contributor

yindia commented Jun 3, 2021

ok

@yindia
Copy link
Contributor

yindia commented Jun 5, 2021

@kumare3 @pmahindrakar-oss checkout influxdb release process. They are using goreleaser for releasing binary with CGO enabled https://github.com/influxdata/influxdb/blob/master/.goreleaser.yml

@kumare3
Copy link
Contributor Author

kumare3 commented Jun 6, 2021

@kumare3 @pmahindrakar-oss checkout influxdb release process. They are using goreleaser for releasing binary with CGO enabled https://github.com/influxdata/influxdb/blob/master/.goreleaser.yml

@evalsocket this is interesting? Why can't we do this - seems simple

@kumare3
Copy link
Contributor Author

kumare3 commented Jun 6, 2021

We can try in this or

@pmahindrakar-oss
Copy link
Contributor

pmahindrakar-oss commented Jun 6, 2021

@evalsocket let us know how it goes with the CGO changes similar to influxdb which you have suggested.

The changes with other library are also almost done aswell which dumps the dot string which can be used in online or commandline renderer for dot files. I will wait for your results to see if we want to change to complete go library for graphviz or use the existing one.

I used this for eg :

https://tinyurl.com/h6m7559j
Note : hyphens in names have been replaced by underscores since graphviz doesn't like those characters. (https://stackoverflow.com/questions/14958346/dot-dash-in-name)

@yindia yindia force-pushed the visualization branch 7 times, most recently from 1846c8e to 03cc7cf Compare June 8, 2021 10:06
@yindia yindia force-pushed the visualization branch 4 times, most recently from 31fefdf to 5b530ac Compare June 9, 2021 18:52
@yindia yindia force-pushed the visualization branch 5 times, most recently from a1440be to 3a929a6 Compare June 10, 2021 13:05
@pmahindrakar-oss pmahindrakar-oss force-pushed the visualization branch 2 times, most recently from d938e10 to 990713a Compare June 17, 2021 08:41
@kumare3 kumare3 force-pushed the visualization branch 2 times, most recently from 3414179 to 990713a Compare June 17, 2021 23:01
kumare3 and others added 13 commits June 18, 2021 14:48
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
* Using go graphviz library to print dot file

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Rebased

Signed-off-by: Prafulla Mahindrakar <[email protected]>
* Linter fixes

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Added some coverage

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Added interface for the mehtods used from graphviz

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Fixed the workflow config flag file and bug fixes

Signed-off-by: Prafulla Mahindrakar <[email protected]>
(cherry picked from commit 77be561)

* Added unit tests

Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Prafulla Mahindrakar <[email protected]>
@pmahindrakar-oss pmahindrakar-oss merged commit 0ea6da6 into master Jun 18, 2021
@pmahindrakar-oss pmahindrakar-oss deleted the visualization branch June 18, 2021 15:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants