Page MenuHomePhabricator

[6and7/n] remove solid from ExecutionStep
AbandonedPublic

Authored by dgibson on Jan 21 2021, 6:25 PM.

Details

Reviewers
alangenfeld
Summary

Replace with SolidDefSnapshot, someday this will be threaded down from above instead of created inline

Test Plan

BK

Diff Detail

Repository
R1 dagster
Branch
rmpipelinedef7
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 21 2021, 7:14 PM
Harbormaster failed remote builds in B24650: Diff 30016!
Harbormaster returned this revision to the author for changes because remote builds failed.Jan 21 2021, 7:59 PM
Harbormaster failed remote builds in B24658: Diff 30027!
Harbormaster returned this revision to the author for changes because remote builds failed.Jan 21 2021, 9:48 PM
Harbormaster failed remote builds in B24664: Diff 30034!
dgibson retitled this revision from remove solid from ExecutionStep to [7/n] remove solid from ExecutionStep.Jan 22 2021, 4:17 AM
dgibson retitled this revision from [7/n] remove solid from ExecutionStep to remove solid from ExecutionStep.

up

dgibson retitled this revision from remove solid from ExecutionStep to [6and7/n] remove solid from ExecutionStep.Jan 22 2021, 3:38 PM
dgibson added a reviewer: alangenfeld.
alangenfeld added inline comments.
python_modules/dagster/dagster/core/execution/plan/compute.py
39–70

these certainly are not memoized - would be good to not land the stack til something on top threads the pipeline snap in to ExecutionPlan building so its available here

python_modules/dagster/dagster/core/execution/plan/step.py
75–76

nit: its a little weird to have methods that are only usable in user mode. Having to pass in a pipeline_def makes it pretty clear, but it would be cool if there was a cleaner way to model it

This revision is now accepted and ready to land.Jan 22 2021, 4:59 PM