[dagit] Auto-refreshing materialization + in progress run overlays on Asset graph #5825

bengotow · 2021-11-30T03:46:09Z

Summary

This PR adds in-progress spinners and status messages to the Asset explorer using @clairelin135's new repo-level graphql query! I also split the query made by the AssetGraphExplorer in half - we load the graph itself (node definitions with their upstream node references) once, and have a separate query that returns the in-progress runs and latest asset materialization for each node. We refresh the latter every 5 seconds to give the asset graph a nice real-time info overlay. My hope is that having a smaller initial query also makes the time-to-usable-DAG lower.

I think a 5 second polling interval is ideal, but if the query is slow on large DAGs it'll back off a bit - I double checked the Apollo source and pollInterval will skip and retry pollIntervalms later if the previous query is still in flight (https://github.com/apollographql/apollo-client/blob/8155890a3ee296799cd318ca2ca749961eb91ce1/src/core/ObservableQuery.ts#L633).

Test Plan

Just tested on hacker_new_assets graph so far.

Checklist

My change requires a change to the documentation and I have updated the documentation accordingly.
I have added tests to cover my changes.

…ality

# Conflicts: # js_modules/dagit/packages/core/src/workspace/asset-graph/AssetGraphExplorer.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/Utils.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/types/AssetGraphQuery.ts

# Conflicts: # js_modules/dagit/packages/core/src/assets/AssetMaterializations.tsx # js_modules/dagit/packages/core/src/graph/PipelineGraph.tsx # js_modules/dagit/packages/core/src/pipelines/GraphExplorer.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/AssetGraphExplorer.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/SidebarAssetInfo.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/Utils.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/types/AssetGraphQuery.ts

vercel · 2021-11-30T03:46:12Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/elementl/dagster/BE7HTXuFPusgWpsk7qp5PYFDDEi3
✅ Preview: Canceled

[Deployment for f940455 canceled]

hellendag · 2021-11-30T16:19:23Z

js_modules/dagit/packages/core/src/workspace/asset-graph/SidebarAssetInfo.tsx

+        >
+          <span>This asset may be materialized soon by </span>
+
+          {['12341514'].map((runId) => (


Placeholder?

Heh whoops, will fix 🤦‍♂️

hellendag · 2021-11-30T16:20:34Z

js_modules/dagit/packages/core/src/workspace/asset-graph/SidebarAssetInfo.tsx

+          <span>This asset may be materialized soon by </span>
+
+          {['12341514'].map((runId) => (
+            <Link to={`/instance/runs/${runId}`} key={runId}>


I think elsewhere we put the link on the ID text or other inner info, rather than the tag itself.

Ahh yeah I can move the link to the ID text 👍

…raph

sryza · 2021-11-30T17:22:26Z

I gave this a spin. It's pretty cool. Some comments:

The "Latest run" spinner in the header appears to still be spinning after the asset spinner has stopped, which is a little confusing. Are we able to get that to auto-refresh as well?
I think "This asset may be materialized soon by Run X" comes off as a little too tentative. Maybe "Run X is currently refreshing this asset" instead?
It would be helpful to add a spinner or similar progress indicator to the asset page, so someone who depends on an asset can discover whether it's in the process of being regenerated, e.g. answer the question "is this just running slow today, or do I need to bother someone about it?".
It feels a little sluggish to wait five seconds for the spinner to show up after launching a run. Would we be able to do some sort of immediate check after launch, and then update every five seconds after that?
For assets that aren't currently being updated but that are pending in an in-progress run, it would be helpful to have some way to represent that they're slated for an update. That might require backend additions though? Could leave that as a separate chunk of work.

…elect all paths from A=>B

bengotow · 2021-11-30T18:17:28Z

That all sounds great! The asset details version of this definitely needs to happen, it'd be super cool - just waiting on one of the two PRs to merge. I think we can address #1 (the latest run header being static) and #2 + #4 today, will get those knocked out!

I think the last point is interesting. Right now, the spinner would be spinning for any asset covered by an in-progress run - the spinner isn't tied to the current step state, just to the current state of the run. Not sure if it'd be helpful to get more granular - we could technically pull not just the current run Ids but the step logs inside those runs? (cc @sryza )

…in-progress

…h-in-progress # Conflicts: # js_modules/dagit/packages/core/src/workspace/asset-graph/AssetGraphExplorer.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/AssetNode.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/types/AssetGraphQuery.ts

…in-progress # Conflicts: # js_modules/dagit/packages/core/src/assets/AssetNodeDefinition.tsx # js_modules/dagit/packages/core/src/assets/AssetView.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/AssetGraphExplorer.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/AssetNode.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/Utils.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/types/AssetGraphQuery.ts

bengotow · 2021-11-30T23:35:34Z

Interesting, it looks like I was wrong! get_in_progress_runs_by_in_progress_step returns step + run IDs only if the step is currently IN_PROGRESS.

@clairelin135 what do you think of breaking this into two states? It'd be ideal if launching a run immediately returned some sort of in-progress state for the launched steps. Right now it takes a while for the spinner to start because the asset you picked actually needs to start, but this granular data is great.

What if we returned both?

inProgressRunsByStep: [{
  stepKey: 'a',
  unstarted: [{runId: 'aaaa'}],
  inProgress: [{runId: 'cccc'}],
}]

Something like this would also be ok as well:

inProgressAssetOps: [{
   run: { id: 'aaaa'},
   unstartedOps: ['a'],
   inProgressOps: [],
}, {
   run: { id: 'cccc'},
   unstartedOps: [],
   inProgressOps: ['a'],
}]

bengotow · 2021-12-01T22:39:45Z

Hey folks! I talked with Claire today about this and it sounds like we'll hold off on the "going to be refreshed by a run, but hasn't been yet" status. This status will address Sandy's points 4 and 5 above - right now there's a delay before the spinner appears because the run needs to launch and begin the step.

The other feedback has all been addressed - there's now live status on the asset details page as well! I think this is ready for another look if you get a minute @hellendag

hellendag · 2021-12-02T17:34:07Z

js_modules/dagit/packages/core/src/assets/AssetMaterializations.tsx

+    if (paramsTimeWindowOnly) {
+      return;
+    }
+    console.log('refetch');


bengotow added 10 commits November 22, 2021 16:07

[dagit] Improvements to AssetGraph performance, "Launch Run" function…

dd03169

…ality

Fix memoization

aa50988

Fix linter issues

89c6c27

[dagit] Reworked asset details page

806bb11

# Conflicts: # js_modules/dagit/packages/core/src/workspace/asset-graph/AssetGraphExplorer.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/Utils.tsx # js_modules/dagit/packages/core/src/workspace/asset-graph/types/AssetGraphQuery.ts

Fix tests

d9258c0

Replace neighbors graph with neighbors list

1f985f4

Fix asOf handling, add disclosure triangles + hover states

5b41c15

Rm tests no longer relevant

4380e4b

Apply black

16138ee

bengotow requested review from prha, hellendag and clairelin135 November 30, 2021 03:46

bengotow force-pushed the bg/graph-in-progress branch from dd5d858 to ee959c9 Compare November 30, 2021 03:59

vercel bot temporarily deployed to Preview November 30, 2021 03:59 Inactive

hellendag reviewed Nov 30, 2021

View reviewed changes

bengotow changed the title ~~Auto-refreshing materialization + in progress run overlays on Asset graph~~ [dagit] Auto-refreshing materialization + in progress run overlays on Asset graph Nov 30, 2021

vercel bot temporarily deployed to Preview November 30, 2021 16:41 Inactive

bengotow added 2 commits November 30, 2021 10:41

Auto-refreshing materialization + in progress run overlays on Asset g…

4b25d48

…raph

Address PR feedback

3f269d5

bengotow force-pushed the bg/graph-in-progress branch from e1a974c to 3f269d5 Compare November 30, 2021 16:42

vercel bot temporarily deployed to Preview November 30, 2021 16:42 Inactive

Merge branch 'master' of github.com:dagster-io/dagster into bg/graph

ae70dc9

bengotow added 3 commits November 30, 2021 11:36

Improve the algorithm for shift-selection, ignore foreign nodes and s…

9c630a2

…elect all paths from A=>B

Switch definition and upstream/downstream in the asset details panel

d353e70

Shrink nodes a bit

8b18163

Merge branch 'master' of github.com:dagster-io/dagster into bg/graph-…

b680c9a

…in-progress

vercel bot temporarily deployed to Preview November 30, 2021 18:19 Inactive

bengotow added 4 commits November 30, 2021 14:14

Thread inProgressRunIds into the asset view

5feaa7d

Only show spinner for runs that have not yet materialized

48c9b2a

vercel bot temporarily deployed to Preview November 30, 2021 22:30 Inactive

bengotow added 3 commits November 30, 2021 17:45

Foreign node CSV should never be considered older than data

0db2615

Make assets with no opName unclickable

c21c23c

Autorefresh the entire asset details page, fix flickering

b887620

vercel bot temporarily deployed to Preview December 1, 2021 00:33 Inactive

Fix linter issues

a096249

vercel bot temporarily deployed to Preview December 1, 2021 00:36 Inactive

bengotow requested a review from hellendag December 1, 2021 22:39

Polish appearance of graphs in the left sidebar of the asset job view

332a047

vercel bot temporarily deployed to Preview December 1, 2021 23:05 Inactive

hellendag approved these changes Dec 2, 2021

View reviewed changes

Rm console log

f940455

vercel bot temporarily deployed to Preview December 2, 2021 19:36 Inactive

bengotow merged commit 5fc0088 into master Dec 2, 2021

bengotow deleted the bg/graph-in-progress branch December 2, 2021 20:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dagit] Auto-refreshing materialization + in progress run overlays on Asset graph #5825

[dagit] Auto-refreshing materialization + in progress run overlays on Asset graph #5825

bengotow commented Nov 30, 2021 •

edited

Loading

vercel bot commented Nov 30, 2021 •

edited

Loading

hellendag Nov 30, 2021

bengotow Nov 30, 2021

hellendag Nov 30, 2021

bengotow Nov 30, 2021

sryza commented Nov 30, 2021

bengotow commented Nov 30, 2021

bengotow commented Nov 30, 2021

bengotow commented Dec 1, 2021

hellendag Dec 2, 2021

[dagit] Auto-refreshing materialization + in progress run overlays on Asset graph #5825

[dagit] Auto-refreshing materialization + in progress run overlays on Asset graph #5825

Conversation

bengotow commented Nov 30, 2021 • edited Loading

Summary

Test Plan

Checklist

vercel bot commented Nov 30, 2021 • edited Loading

hellendag Nov 30, 2021

Choose a reason for hiding this comment

bengotow Nov 30, 2021

Choose a reason for hiding this comment

hellendag Nov 30, 2021

Choose a reason for hiding this comment

bengotow Nov 30, 2021

Choose a reason for hiding this comment

sryza commented Nov 30, 2021

bengotow commented Nov 30, 2021

bengotow commented Nov 30, 2021

bengotow commented Dec 1, 2021

hellendag Dec 2, 2021

Choose a reason for hiding this comment

bengotow commented Nov 30, 2021 •

edited

Loading

vercel bot commented Nov 30, 2021 •

edited

Loading