-
Notifications
You must be signed in to change notification settings - Fork 112
Workflow visualizer prototype #1363
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
Conversation
13a3454 to
42e3b9d
Compare
| val trace = workflowAdapter.fromJson(jsonString) | ||
| ParseResult.Success(trace) | ||
| val workflowAdapter = createMoshiAdapter() | ||
| val unParsedTrace = workflowAdapter.fromJson(jsonString) |
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.
nit: this is really jsonFrames or something like that right? In other words, the 'list' part of it has been parsed out, just not the tree of each frame.
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.
In fact, i think this has actually done all the parsing into Node? It's just we have not built it into a tree. So these need renaming.
really this is the parsedTrace, and below parsedTrace should be frames (the frames as trees?)
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.
this is a good point, but on another branch, I have 3 different structures going on:
affectedNodes: List<Set<Node>>: This contains a set for each render pass, so I can easily access and decide which nodes need to be highlighted within the larger tree. If there is too much I'm passing around between functions, I can easily remove this and use a recursive algo to find the nodes I need to highlight.
parsedTrace: List<Node>: This would be a formed tree that only contains information of the render pass
frameTree: List<Node> (this is poorly named): This would be a continuously growing tree that contains every workflow that was ever started
I would agree that from that everything should be parsedSomething, though. Maybe like parsedRenderPasses for the moshi call, and then parsedFramesSet, parsedFrames, mainTree for the 3 data structures? Or maybe I need to change the naming scheme...
| } | ||
|
|
||
| /** | ||
| * We take an unparsed render pass and build up a tree structure from it to form a Frame. |
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 take an unparsed render pass and build up a tree structure from it to form a Frame. | |
| * We take an unparsed render pass and build up a tree structure from it to form a Frame. | |
| * @return Node the root node of the tree for that frame. |
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.
Done
| */ | ||
| private fun getFrameFromRenderPass(renderPass: List<Node>): Node { | ||
| val childrenByParent = renderPass.groupBy { it.parent } | ||
| val root = childrenByParent["root"]?.single() |
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.
let's extract this to a constant so its clear (you can create a companion object with a const val ROOT_ID: String = "root" or something like that.)
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.
Done
| * We take an unparsed render pass and build up a tree structure from it to form a Frame. | ||
| */ | ||
| private fun getFrameFromRenderPass(renderPass: List<Node>): Node { | ||
| val childrenByParent = renderPass.groupBy { it.parent } |
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 are using ids for the parent not names right? Otherwise this won't be unique.
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's directly referencing the parent Node, which are compared by id's, so yes. Unless you are asking about something else?
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.
Done
| * Recursively builds a tree using each node's children. | ||
| */ | ||
| private fun buildTree(node: Node, childrenByParent: Map<String, List<Node>>): Node { | ||
| val children = (childrenByParent[node.name] ?: emptyList()) |
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 think we need to use node.id as the parent identifier, otherwise it won't be unique!
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.
Done
10c105d to
c7890aa
Compare
| The root workflow Node uses an ID of 0, and since we are filtering childrenByParent by the | ||
| parentId, the root node has a parent of -1 ID. This is reflected seen inside android-register | ||
| */ | ||
| const val ROOT_ID: String = "-1" |
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.
My guess is when the parent is null in the WorkflowSession you have just given it a default value of -1 in your recording code?
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.
Yep! I will make note of this on both PR's in workflow library and register
Since ActionLogger will only be responsible for logging, the parsing part will be done here, so it involves slightly more complicated logic by building the tree structure from the ground up
…when building tree
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
8981c4d to
2f4f69a
Compare
First prototype of creating a workflow visualizer.
Changes were made to how a given JSON file is parsed due to a difference in how logs were being stored.
The Node model also needed to be changed to match any additional information being passed in
WorkflowInterceptor.WorkflowSession.sessionIdis unique, and the fields of the same Workflow Nodes can differ, we only use sessionId as a form of comparison.