-
Notifications
You must be signed in to change notification settings - Fork 198
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
Add REST endpoint to get DOT graph of a job #242
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! What generates the png?
|
I didn't think this was possible, but it looks like someone has transpiled graphviz to wasm so it runs in the browser now https://github.com/magjac/d3-graphviz. |
very nice graph |
@yahoNanJing @thinkharderdev @avantgardnerio This is now ready for review. I will follow up with smaller PRs to improve formatting and add metrics. |
Ok(state) | ||
} | ||
|
||
fn write_plan_recursive( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use/move something similar to DataFusion (minus the stage part, currently).
LGTM |
// truncate after translation because we know we only have ASCII chars at this point | ||
// so the slice is safe (not splitting unicode character bytes) | ||
if let Some(limit) = max_len { | ||
if sanitized.len() > limit { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use String::truncate
?
sanitize(str, Some(100)) | ||
} | ||
|
||
/// Make strings dot-friendly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK labels can be quoted like this
"Thïs įs a 🦄 and a \" quote"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So surround by quotes and escape quote character is all that's needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did try this, and it failed, but maybe I made a mistake ... I will try again in the next PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments but looks great 😃
Thanks for the review @Dandandan and @yahoNanJing |
Which issue does this PR close?
N/A
Rationale for this change
This is essential to comprehending how Ballista is executing a query.
What changes are included in this PR?
New endpoint:
http://localhost:50050/api/job/<job-id>/dot
Are there any user-facing changes?