Feature/Directed Asyclic Graph #241

just-at-uber · 2021-01-15T20:14:53Z

This is a new feature which interprets the history of a workflow and displays as a Directed Asyclic Graph (DAG).
Majority of this work was completed by @codemma so a big thanks to her contribution.

Features:

The graph can be toggled on by clicking "Show graph" button.
This graph will allow users to navigate the history via the graph:
- Selecting a node in the graph will scroll the history list to that event.
- Selecting an event in the history list will navigate to the node in the graph.
Navigating to child workflows is as simple as selecting the child node in the graph and click "to child" button.
Navigating to the next execution (if workflow continued_as_new), users can select the end node of the graph and click "next execution" button.
Users can navigate back to the previous execution or the parent depending on which workflow they have navigated to using the "to parent" button or "previous execution".

Some caveats to be aware of:

Performance of rendering large workflows can crash the browser. There will need to be a limit on how many history items are allowed before showing a warning message to the user (will be done in a separate PR).
Not all history event types have been implemented or tested thoroughly so it is possible that some graph nodes do not behave how they are intended to be.
Code has not been unit tested. Ideally this would be part of this PR however it will require more work and want to get this out to users as soon as we can for feedback. I have added a task to our backlog to come back to this in the future.

Screenshots

Child workflow example

Selecting child node

Selecting end node of a child workflow

Selecting end node of within a set of executions

* added simple route to tree graph * added new component for workflow graph * now displaying a svg circle, added some styling * added a simple line generator to display grah * moved code from js project to graph vue component, added new data in data.json * removed old file, fixed click issues by a temporay hack' * trying to integrate cytoscape graph@ * removed and changed some imports to make cytoscape graph work * updated store import * added loading of workflow to history component for testing * removed typescript files * added new test data * added demo-data * added store * added event function map but in pure js * added old tree component which will be updated * changed name of store * updated npm package of cytoscape to 3.16.0 * renamed tree vue file * changed to receive events as props@ * history now loads workflow container instead of graph * modiefied graph container to receive new props and set a small delay of displaying graph * changed with and height of graph to fix problem with zoom * now looping over events instead of workflow@ * added a new event map to get event info * changed all eventtypes to map to servers json format * updated store to receive both workflow id and child id * updated graph component to send correct data to store * added a function to set route to child * changed the way childworkflow route is set in event map * changed how parent is routed to * changed variable name for parent route * removed old function for routing * removed home btn * added a simpler way to change the route@ * added query to route to route to the graph format and not grid * renamed showGraph variable to showTimeLine * renamed variables and functions from showGraph to showTimeline * toggling with flag instead of true false variable * added a function setWorkflow which rezises the split window based on the variable graphView * removed old link to tree graph * added a refresh btn which forces a reload of the graph container * sending isworkflow running to the history component from index * refresh functioning, only on mount needs to bw fixed * removed debugging msg * Refresh btn now only shows if there is more data to reload * only sending the prop (events) to graphView if it has been fetched and has length * removed redundant code and changed variable names to better represent their functionalities * revomed debugging code * displaying graph on right side works but is hacky - needs to be fixed * added flexbox to the container when the dag graph is displayed to layout the slider on the right * added functionality which check route query to determine how to split the window@ * new name for class to match current class naming style * removed commented code and console log * changed to listen to query updates rather than store to update currently selected node * removed node list from graph and not used function * removed commented code * added new function zoomtonode which is called on mount, if a eventId exist on query * removed old console log * removed not used method in store set rendered nodes * removed functionality regarding rendered nodes (only relevant when we have a node list to the graph * removed old console logs * removed another debug console log * added omit to router query when routing from/to child to avoid trying to display a node with same eventId which doesnt exist@ * added a check to see if eventId is definied on graph mount * update route query when a node is clicked * added a watch function which watches changes in event id query and then scrolls the grid to that event * removed old on click functioanlity and added new to allow deselection of node * removed eventlist and moved childbtn to topbar * removed event list and all css linked to it and node list * added and extra if statement to check if you have already clicked on bg * removed old getters and setters in vue store * added functionality to check if a node is clicked or not * added matching top bar to canvas (same as grid) * added new btn styling to match rest of the page, using the predefinted mixins@ * added proper buttons for parent and child routes@ * removed unecessary css code * styled refresh button * added a check to enableSplitting to not do it on graph view * removed clickinfo from eventmap * updated name of event info and removed click info * removed old static workflow loading functionality and its corresponding data and props * removed old event map functions and renamed the current one * removed old code and renamed some variables * removed old demo data * changed child btn text to To Child * changed variable names for better readability. Added overflow ellipse for workflow name text * added new variables names to store for better readability regarding childworkflow * added functionality to show child btn for previous execution run id * functionality for changing the name of the parent btn, and only use run id as route * updated continued as new event to return run ID * added a new mutation to update route for new executions * Change cytoscape version * added functionality to make graph display child btn on grid click * refractored current code for better readability * removed onNodeClick function which is not necessary anymore * added an extra check to make sure refresh btn is not showed unecessarily * updated even connection functionality to not break for different chunk sizes * removed old console log * removed legend imports in graph component * added legend to graph container * added graph legend component and changed its styling a bit * small html and css fixes * moved legend to bottom of graph * removed chunckWorfklow and code that is not used * removed old graph file * fixed pr comment on this=self, and simplified zoom * removed unused import * passing eventId as prop instead of listening to changes in query * removed unnecessary async prefix for viewInit * update to check for prop and not for route query * changed from let to const for zoom lvl * added map mupations to connect to store * resolved merge conflicts * added demo data to gitignore * commented out demo data * making sure cytoscape version stays on 3.16.0 * added a new file for calculating the layout using timestamps * added a new file which keeps track of events and nodes and builds the graph * removed unused function * Order nodes by timestamp in graph visualization * Enable chrono edges * Avoid overlapping edges * Apply lint --fix * Disable chrono edges for large workflows * Fix timeline elements test * Use showGraph url parameter in tests Co-authored-by: Aliaksei Talkachou <aliaksei@uber.com>

* Refactor workflows list page (#171) * Refactor workflow list for reusability * Add workflows view showing all workflows (#173) * Fix workflows view console error for empty result (#186) * Avoid possible duplication of workflow instance in ALL view (#187) * Fix workflows all view (#208) * Fix workflows ALL view showing empty space (#239) * Fix workflows date range filter dropping out running workflows (#240) * Adjust workflows view spacing (#241) * fix lint & attribute temporal * adding back in default range * Show open worfklows first in All view (#243) * fixing lint * attributing temporal * fixing bugs * fixed bug with change status * fixed issue with overlapping scrollbars * fixing bug where fetch was calling with empty next page token * fixing alt colours in list. sync initial calls incase of race conditions. fixing lint * rename workflow-grid. Fixed integration tests. Fixed bug where range was not being set correctly from URL. * fix lint * rename onScroll * adjust ordering Co-authored-by: Ruslan <11838981+feedmeapples@users.noreply.github.com>

vytautas-karpavicius · 2021-02-10T07:55:25Z

client/routes/workflow/helpers/get-default-split-size.js

+
+const getDefaultSplitSize = ({ graphView }) => {
+  switch (graphView) {
+    case 'dag':


Should those dag timeline be constants as well? I see such strings occurring many times in this PR.

vytautas-karpavicius · 2021-02-10T08:01:38Z

client/routes/workflow/history.vue

+      }
+    },
+    toggleShowTimeline() {
+      if (this.graphView === 'timeline') {


Having a graphView we are essentially limiting showing either timeline OR dag. This is probably fine as both would take too much screen space. But maybe it is better to give this choice to the user and leave flexibility to have both, at least from code perspective.

I think this can be done however it would mean we would need two separate sliders to control which adds a lot more complexity. Perhaps we can add a task in the backlog and prioritize this against other work?

Created https://t3.uberinternal.com/browse/CDNC-1057 to reflect work needed to complete this task.

vytautas-karpavicius · 2021-02-10T08:10:21Z

client/routes/workflow/components/workflow-graph/index.vue

+    reloadWorkflow() {
+      this.eventsSnapShot = this.events;
+      this.isGraphLoading = true;
+      this.forceRefresh = !this.forceRefresh;


This looks suspicious. Why toggling is needed here?

I think whats going on here is its forcing the graph to refresh by toggling the boolean flag.

Reading this article shows two accepted ways of forcing a re-render:
https://michaelnthiessen.com/force-re-render/

This shows one of the methods which is using the key method. In the example it uses an integer to increment to larger values each time, here we toggle between true and false. We could change this to increment the counter like the example. Alternatively we could use this.$forceUpdate(); which will force the entire component to re-render (not just the graph).

Let me know your thoughts and I will go with that approach.

Thanks!

Also the reason why we are forcing a refresh is I think the graph component is not reactive as it is built using a jQuery library.

Ahh, ok understood. No need to change it, this is elegant enough.
You could add a comment with the link explaining the technique though. Could be helpful for those not familiar with it.

Comment added. Thanks.

vytautas-karpavicius · 2021-02-10T08:21:48Z

...ow/components/workflow-graph/helpers/get-event-connections/activity-task-cancel-requested.js

+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+const ActivityTaskCancelRequested = event => ({


Wondering whether is a common practice to split things into so many small files in the front-end development. In general I think it is a good thing, but this looks like an overkill to me.

This was previously in one big file (300 lines long or longer). We can put back into one file if you like. My thoughts on this was we could individually unit test each piece. Also searching for an existing EventType is easier because it is all alphabetical by default. Also adding a new event type is easier as you don't care about ordering as this is handled by the file system. I don't really mind either way.

codemma and others added 13 commits November 6, 2020 11:26

Merge branch 'master' into feature/dsl-graph

c5f2a1e

removing git ignore file

72236cb

updating package-lock and updating ordering in cytoscape component

d4b40ef

refactoring get-event-connections. fixing lint. adding missing licenses.

2eab4dc

minor refactorings

57e9d1c

minor refactoring

7167088

updating start date in license headers

36351a5

moving files to a more logical structure.

fa77ca9

small refactor

0902470

small refactor for legend

1e3b5cd

adding license header

22f143f

fixing lint

0e7f658

just-at-uber added 16 commits January 27, 2021 10:32

moving constants

a40623d

Merge branch 'master' into feature/dsl-graph

affa6eb

small rename refactor for graphView values.

ea64c78

adjusting line in legend

5ff2add

moving constants out of helper into constants file.

bf1215f

moving comments for constants.

a84bf56

moving more constants out

5dd2832

reordering constants

7e46454

minor refactoring. removing some warnings in console.

ce5cb51

adjusting whitespace in legend

ab093fe

fixing lint

ce603eb

minor refactoring of CytoscapeLayout for better readability.

4c2a50b

minor refactor

a351a1a

minor refactoring

1155d66

adjusting ordering

1815f28

breaking apart cytoscape layout into smaller helpers

c6e0dec

just-at-uber added 15 commits February 3, 2021 17:05

adding license headers

554e636

fixing lint

8f08ccd

removing unused prop

e0958ce

moving to computed prop

1797ea0

moving constant. moving to computed field

da0a93d

simplifying code

6bd9f6d

reference helper

d6ad330

removing unused variables

535e23a

moving out more constants

9c172d0

separating get event connections into smaller files.

e4ce307

minor refactoring

78cd901

moving more constants out

194be99

fixing lint

cec818b

updating comment

60341d8

Merge branch 'master' into feature/dsl-graph

a6c8268

just-at-uber changed the title ~~Feature/dsl graph~~ Feature/Directed Asyclic Graph Feb 5, 2021

just-at-uber marked this pull request as ready for review February 5, 2021 18:55

just-at-uber requested review from a team and vytautas-karpavicius February 5, 2021 21:05

just-at-uber added 3 commits February 9, 2021 10:24

tweaking vuex store to have graph namespace for application separation.

38b5bbc

fixing lint

479c4b4

removing direct import reference to store in components.

7e6b41c

vytautas-karpavicius reviewed Feb 10, 2021

View reviewed changes

addressing PR comments

d0b5e51

just-at-uber requested a review from vytautas-karpavicius February 10, 2021 18:52

vytautas-karpavicius approved these changes Feb 11, 2021

View reviewed changes

added comment in code.

a448693

just-at-uber merged commit 1903522 into master Feb 11, 2021

just-at-uber deleted the feature/dsl-graph branch February 11, 2021 19:13

just-at-uber mentioned this pull request Feb 25, 2021

v3.23.0 #258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/Directed Asyclic Graph #241

Feature/Directed Asyclic Graph #241

just-at-uber commented Jan 15, 2021 •

edited

Loading

vytautas-karpavicius Feb 10, 2021

just-at-uber Feb 10, 2021

vytautas-karpavicius Feb 10, 2021

just-at-uber Feb 10, 2021

just-at-uber Feb 10, 2021

vytautas-karpavicius Feb 11, 2021

vytautas-karpavicius Feb 10, 2021

just-at-uber Feb 10, 2021

just-at-uber Feb 10, 2021

vytautas-karpavicius Feb 11, 2021

just-at-uber Feb 11, 2021

vytautas-karpavicius Feb 10, 2021

just-at-uber Feb 10, 2021

Feature/Directed Asyclic Graph #241

Feature/Directed Asyclic Graph #241

Conversation

just-at-uber commented Jan 15, 2021 • edited Loading

Features:

Some caveats to be aware of:

Screenshots

Child workflow example

Selecting child node

Selecting end node of a child workflow

Selecting end node of within a set of executions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

just-at-uber commented Jan 15, 2021 •

edited

Loading