Page MenuHomePhabricator

cleanup memoization copy_required_intermediates_for_execution
ClosedPublic

Authored by yuhan on Wed, Jan 13, 1:45 AM.

Details

Summary

as we are moving towards IO managers, we don't need copy_required_intermediates_for_execution to be run before step execution

this diff moves the the intermediate object copy check inside the intermediate object adaptor.
when loading inputs, the adaptor will check if the source is from upstream_output and source run id is the same as parent id, if so, it will copy the object before loading.

besides, similar to D5474, we switch CP_OBJECT event to context.log the file path as log messages, so then we can clean up ObjectStoreOperation events

Test Plan

bk

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Wed, Jan 13, 2:19 AM
Harbormaster failed remote builds in B24221: Diff 29466!
python_modules/dagster/dagster/core/execution/context/system.py
299–301

use execution_plan.step_handles_to_execute as the source of truth for steps to execute, because one can execute a sub plan directly without pipeline_run.step_keys_to_execute like https://sourcegraph.com/github.com/dagster-io/dagster/-/blob/python_modules/dagster/dagster_tests/core_tests/execution_plan_tests/test_execution_plan_reexecution.py#L80:24

yuhan requested review of this revision.Wed, Jan 13, 2:48 AM

That was quick! Love the big red portion.

This revision is now accepted and ready to land.Wed, Jan 13, 4:56 PM