Page MenuHomeElementl

[dagit] Refactor components with subscriptions
ClosedPublic

Authored by dish on Jul 27 2021, 6:09 PM.

Details

Summary

There are three components that use DirectGraphQLSubscription. Refactor these to be SFCs. The next objective here will be to grab more information about the WebSocket connection from context: namely, connection parameters that could be defined at the application root. These would currently be ignored because the DirectGraphQLSubscriptions are independent of the WebSocketLink set up in AppProvider.

Next steps:

  • Modify WebsocketStatusProvider to supply more values intended for WebSocket usage
  • Move the SubscriptionClient in AppProvider to WebsocketStatusProvider
  • Delete WebSocketLink from AppProvider, because we don't really use it
Test Plan

View runs in Dagit, start a few. Verify that WebSockets are set up correctly, receive messages as intended, and results are rendered properly as they arrive.

Start Dagit with grpc, restart grpc. Verify that the repository location observer shows the appropriate message in the left nav.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dish requested review of this revision.Jul 27 2021, 6:19 PM

This looks good! I could have sworn there was more of a reason these were class components (shouldComponentUpdate or something like that...) but it doesn't seem like it. The new SFC implementations look equivalent to me sans that one setNodes([]) call 👍

js_modules/dagit/packages/core/src/runs/LogsProvider.tsx
131

It looks like this no longer calls ` this.setState({nodes: []});` before switching the subscription to a new runId. I'm not sure that this SFC is ever given a new runId after it's mounted in practice, but would be good to keep in here!

This revision is now accepted and ready to land.Jul 27 2021, 6:53 PM

setNodes() when runId changes