Gah you're right

Superseded by https://dagster.phacility.com/D4806

Would you eliminate @solid?

I think there are almost no cases where SolidDefinition is preferable to @solid, because solids wrap functions. Unless we strongly guide them otherwise, I think some people might prefer to use ModeDefinition(resource_defs), because they might find putting their resource_defs inside a function weird.

I think this is a net positive. My two reservations would be:

• Two ways of doing something is worse than one.
• Operating on a function instead of a resource dict gives users an extra degree of flexibility that they might find confusing.

up

up

I like the result!

Quick turnaround! I like the direction this has headed in a lot. I think the AssetStore API makes a ton of sense, and I like how it's threaded through.

up

We discussed this a bit on this thread: https://threads.com/34386850200

This LGTM. As chatted about in Slack, we should get approval in the #proposed-api-changes channel.

up

up

Awesome stuff - just want to make sure I understand the breaking correctly here: A call such as Output("value", address="some_address") is no longer permitted?

I might not be understanding the issue correctly, but I actually think the current behavior might be the correct behavior. If we have two steps that are writing to the same address, then the step that writes last will presumably be overwriting what was written by any earlier steps. Which would mean that it's incorrect to indicate that we've cached the results written by the earlier steps.

Nice! This implementation looks very straightforward. I thought it would need to be more complicated.

This seems straightforward enough. Any idea why we went with NullPool in the first place?

LGTM

Consolidating get/set_intermediate with get/set_intermediate_to/from_address seems like the right move to me. These aren't public APIs, so I'm not worried about the change.

up

up

up

Going forward, ObjectStoreOperation sounds only related to object_store. how about create a IntermediateOperation (or maybe AssetOperation) event which has information of either ObjectStoreOperation or AssetMaterialization? - so we could consolidate all the set/get operations and merge all different ways to handle intermediate (both in default intermediate basedir and externally stored through object_store or type materializer/loader)

up

up

up

Problem from last time: https://dagster.phacility.com/P98

up

up

up

As discussed with Johann, if we can get away with avoiding this fanciness and having the user define a custom resource, I think that's preferable. If this becomes a widespread request and source of pain, then worth considering building this into the pyspark_resource at that point.

up

Have you checked to see whether this gets the test perf back into a reasonable range?

Thanks for taking this up. This looks good to me. Have you checked to see whether this gets the test perf back into a reasonable range? This cuts out generating the execution plan for every output, but it still resolves versions for every output, so wondering whether it's worth a followup to rein that in.

up

LGTM!

This seems reasonable to me. Had mostly minor stylistic / readability comments.

up

up

up

up

up