this is a lot better in terms of reducing grpc-specific logic in dagster-graphql.

In terms of separation of concerns though it still feels like the graphqlcontext is more aware of the specific implementation of some of its handles than it needs to be?

I think this is mostly a naming thing - but I'm wondering if this should be expressed in terms of the context listening to state changes in all of its locations, without any reference to gRPC specifically? There's actually nothing grpc-specific about how it reacts to these events - if the location goes down or indicates that it has updated , we reload it (the fact that it's the gRPC server going down specifically is an implementation detail of that specific handle - you could imagine reloading a docker container in a similar away because something docker-specific caused it to go down or update).

I think if we replaced

so if we remove the instance check and make it add_state_subscriber (with no reference to grpc) - and then most handles don't actually emit those events, this class would no longer have anything gRPC specific in it?

so if we wanted to make these events less gRPC specific they could look something like

LOCATION_UPDATED

LOCATION_DISCONNECTED (or LOCATION_UNREACHABLE if you want to go even more generic, but meh)

LOCATION_ERROR

suggestion is to put these on the base class, makes the names not gRPC specific, but have them be a no-op on most classes