I'm curious as to whether yall think this could potentially be a problem for other types of resources (ie reconstructing them all isn't the right approach), or if the way we conceptualized resources has fundementally changed as a result of us moving previously separate concepts to be resources. I'd imagine as we make more and more stuff resources, this might be a problem in other cases as well.

IMO it is more of the question how we view resources conceptually. My understanding to the current resources is that they are all definitions, meaning every time we run a pipeline or reconstruct the context, it would be a new instance of a resource definition.

I think it could potentially be a problem if we push more usage to resources, esp in some cases users may want to keep the states inside a resource instance cross pipeline runs.

@sandyryza @schrockn thoughts?

I think the core problem here is the reconstruct_context is a flawed concept.

In order to do testability right now I think we should do a pattern like this: https://github.com/dagster-io/dagster/discussions/3329#discussioncomment-146413 where you pass in the resource instance in a way that is accessible during the computation.

If I were to go back in time, the in-memory execution variants would look something like:

with create_context(...) as context:
   run_graph(context, graph, ....)
   assert context.resources.whatever

or something similar.

In short: I don't think we should try to make in memory instances work with reconstruct_context

Strongly agree that the context manager approach would be the ideal.

That said, I'm concerned about breaking users who are doing something like:

@solid
def my_solid(_):
    return 5

@pipeline
def my_pipeline():
    my_solid()

result = execute_pipeline(my_pipeline)
assert result.result_for_solid("my_solid").output_value() == 5

E.g. I suspect we do this in tests all over the place.

Should we just do a more hacky check here instead of threading through resource_instances_not_to_reinit? If we are only going to use this for the mem_io_manager then it is more surgical just to hardcode a check here

we will still need to thread the mem_io_manager object through tho. so it would be, instead of resource_instances_not_to_reinit which requires adding the do_not_reinit arg on ResourceDefinition (hacky too), we only pass mem_io_manager_instance through?

ah i misread this. yeah you need this. hmmmm

[3]