Page MenuHomeElementl

[dagit] Extract app configuration
ClosedPublic

Authored by dish on Mar 29 2021, 10:53 PM.

Details

Summary

We currently pull a handful of static configuration values out of global space in Dagit, at various points throughout the app.

Instead, provide these at a single point (index.tsx) and make them available via context. This gives us a place to define top-level configuration that can be made more flexible over time.

Also create the Apollo client within the scope of the AppProvider, feeding it configuration as needed.

Test Plan

BK, load Dagit and sanity check by navigating through the app.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dish requested review of this revision.Mar 29 2021, 11:02 PM

This looks great to me and I think this is a good pattern to set for these sort of global config values going forward. I'm actually surprised we didn't have more hardcoded stuff, 3 variables is not bad!

js_modules/dagit/packages/core/src/app/AppProvider.tsx
140

Haha contexts for days ๐Ÿ˜…

js_modules/dagit/packages/core/src/nav/LeftNavRepositorySection.test.tsx
58

I wonder if it'd make sense to export an AppProvider.Test from the AppProvider.tsx file? It'd be nice to have AppProvider and the test version of AppProvider side by side since new contexts likely need to go in both with different values?

This revision is now accepted and ready to land.Mar 29 2021, 11:08 PM

Make a test provider for AppContext

This revision was automatically updated to reflect the committed changes.