-
-
Notifications
You must be signed in to change notification settings - Fork 258
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 a dependency graph tool. #1132
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.
LGTM for b337439. Really cool!
|
||
|
||
class Graph(OutputMixin, Command): | ||
"""Generates a dot graph of the dependencies contained in a PEX file.""" |
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, generating a graph is definitely useful, but if this were to emit JSON, someone could fairly easily convert to DOT if they wanted to, and JSON is easier to parse if you want to do something other than render it.
Is there already a tool to dump the dependencies in a parseable format? That might be a higher priority.
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.
It would also be nice to have a standard format that Pants can render in various ways, so that it's uniform across languages (when that's a thing).
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.
There is no such tool. I'm happy to circle back and add support for dumping some json format instead of DOT, but DOT was immediately a priority for me working on the resolve-from-pexes project.
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'll point out DOT is that standard format https://graphviz.org/doc/info/lang.html - there are just not alot of parsers pre-written for it. There is one for Python though https://pypi.org/project/pydot/. All that said, I'll re-iterate I'm happy to circle back when Pants identifies a suitable alternate standard.
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'm happy to circle back and add support for dumping some json format instead of DOT, but DOT was immediately a priority for me working on the resolve-from-pexes project. [...] I'll point out DOT is that standard format https://graphviz.org/doc/info/lang.html - there are just not alot of parsers pre-written for it.
If the key driver is to be able to parse it, and being able to visualize it is secondary, it would seemingly take less code to dump an adjacency list in JSON (no standard necessary, I don't think).
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.
The pydot project does the parsing of DOT format and provides a Graph with get_edges, etc. As to an adjacency list in json being simple - agreed. So today I do that here in PEX and tomorrow you need this from coursier. Will it produce the same format? Do we need to hack on it to do so? Looking into some standard and then reporting back to the PEX project to influence its choice seems wise instead of ginning up something ad-hoc. At least DOT has a spec.
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.
And to highlight again the above - the key driver for me (and Pex) here was to look at the graph to help sort through the resolve-from-pex project. Pants may have other needs and that's fine - proposing a standard with deliberation would be great if that's what Pants wants to lobby for.
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 do not think that you need a standard here (and a spec as large as DOT's is arguably counterproductive: how much of the DOT spec would you have to implement to conform?) Each set of @rules
that wraps a resolver would just need to emit an adjacency list parsed from whatever format was natural for those rules... you would not need an abstract file parser, just an in-memory data type for the adjacency list.
As to an adjacency list in json being simple - agreed. So today I do that here in PEX and tomorrow you need this from coursier.
It already supports JSON output: https://get-coursier.io/docs/json-report
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.
To clarify, I think it's fine to merge this as-is and revisit as 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.
Here - I filed the initial feature request for you: #1137 It needs some filling out though with the actual requirements Pants has unless Pants truly just wants an adjacency list with no extra data.
By default this renders svg which is useful since it includes clickable links to the PyPI page for each node when viewed in a modern browser.
By default this renders svg which is useful since it includes clickable
links to the PyPI page for each node when viewed in a modern browser.