Might be good to split the diff into two or three: first two (AssetStoreOperation -> HandledOutput & LoadedInput) + do a full pass on ObjectStoreOperation deprecation

Yeah, agreed that there's a lot going on here. I split out the addition of HandledOutput and LoadedInput into a separate diff: https://dagster.phacility.com/D5519.

The reason I included the AssetStoreOperation and ObjectStoreOperation changes together in this diff was that I didn't want to further complicate the logic inside execute_step/ core_dagster_event_sequence_for_step.

What's your plan of attack with regards to ObjectStoreOperation and the intermediate storage wrapper? Are you planning to preserve ObjectStoreOperation?

Im preserving ObjectStoreOperation for the first two diffs on my stack - so when i rebased off of this diff, it complicated my logic lol
I think not to further complicating execute_step logics makes sense here, but let's make it clear that ObjectStoreOperation is still some where in the code space and we will remove the rest on my end :)

Overall I think the idea of replacing ObjectStoreOperation with HandledOutput and LoadedInput makes sense, but I think now we should hold it off until HandledOutput and LoadedInput log the same information as ObjectStoreOperation, i.e. file path.

Looks almost there!

The pattern that we had with ObjectStoreOperation where we put the obj on it and then call serializable to strip it out is a little confusing. Now that the user code isn't responsible for generating the event, do we need this pattern anymore?

A couple potential ways of implementing this:

• Add a get_load_event method to StepInputSource which returns an event with the appropriate params.
• Have load_input_object return a LoadResult, which could be a tuple of LoadedEvent and obj

I like LoadResult

@sandyryza re:

The pattern that we had with ObjectStoreOperation where we put the obj on it and then call serializable to strip it out is a little confusing. Now that the user code isn't responsible for generating the event, do we need this pattern anymore?

without this pattern, we would involve DagsterEvent in the inputs.py.

One thing im not sure about (which is why im creating this LoadedInput event def) is whether we want to consolidate all the yield DagsterEvent.some_event() event emitting only in core_dagster_event_sequence_for_step in execute_step.py. I see this is the current pattern. But if it's fine to yield DagsterEvent in load_input_object in input.py as well, i can switch back to the initial approach which yield event or value from load_input_object.
cc @alangenfeld ^regarding the core event sequence for step code path.

oops had some old inlines i forgot to submit too

we want to consolidate all the yield DagsterEvent.some_event() event emitting only in core_dagster_event_sequence_for_step in execute_step.py. I see this is the current pattern.

This doesn't seem critical to me, you'll still have an isinstance check in step.py so I don't think you are losing much understandability