This diff updates the Dagit webserver such that the DagsterGraphQLContext is request scoped and clones the current workspace is at the top of the request. This is needed due to the introduction of the repository location watch threads that can modify the context. Now the watch threads only run on the long-lived global context.
I think we should try to be a bit more clear and explicit with this set up - since I expect us to compound on it over time. Instead of this root_context stuff, lets have two different classes (no inheritance preferably), one for process scope and one for request scope. A quick search shows a limited number of DagsterGraphQLContext so renaming in same diff should be fine, not sure the DagsterGraphQL prefix is necessary. Ideally the request scoped context is an immutable namedtuple. You shouldn't need all the getters on the process scoped one
I think we should try to be a bit more clear and explicit with this set up - since I expect us to compound on it over time. Instead of this root_context stuff, lets have two different classes (no inheritance preferably), one for process scope and one for request scope.
i expect the class split should make this possible to model more cleanly
^ this invocation should just return the new object directly
That makes sense, great call. Just to be sure I'm understanding this correctly, does that mean most of the graphql tests should be updated to be against the request scoped context?
Actually I'm realizing it doesn't make much sense for the request context to have a reference to the process scoped context since we don't want the process-scoped threads to be running for each request
Yea I expect most existing call sites should be againts the request scoped one. You might end up with like a for_test static method to construct an instance or something.