Page MenuHomePhabricator

mappable impl
Needs ReviewPublic

Authored by catherinewu on Oct 28 2020, 3:51 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Test Plan

bk

Diff Detail

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

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 28 2020, 4:18 PM
Harbormaster failed remote builds in B20308: Diff 24639!
Harbormaster returned this revision to the author for changes because remote builds failed.Oct 28 2020, 6:11 PM
Harbormaster failed remote builds in B20330: Diff 24663!
Harbormaster returned this revision to the author for changes because remote builds failed.Oct 28 2020, 6:37 PM
Harbormaster failed remote builds in B20336: Diff 24670!
Harbormaster returned this revision to the author for changes because remote builds failed.Oct 28 2020, 7:05 PM
Harbormaster failed remote builds in B20342: Diff 24676!

support dagit/graphql "retry from failure" use case

prevent graphql error that happens during re-exec

no show stoppers i can see - figuring out a clean way to do what is currently handled by deconstruct_step_key will be an interesting challenge

python_modules/dagster/dagster/core/definitions/output.py
99–103

nit: is_mappable not a constructor arg but just result of overridden method

python_modules/dagster/dagster/core/events/__init__.py
203–204

would expect this to go be in event_specific_data

python_modules/dagster/dagster/core/execution/memoization.py
63–67

one value of having separate types is failing check.inst calls when mappable things show up where they are not expected

python_modules/dagster/dagster/core/execution/plan/objects.py
171–243

think there is some value in modeling the unresolved input more clearly as a separate thing - this class is already a bit confusing and having the mappable / resolve like this makes it even more confusing

370

definitely weary of using inheritance here

python_modules/dagster/dagster/core/execution/plan/plan.py
307–308

will have to come up with a clean way to model this all and update callsites accordingly