Page MenuHomePhabricator

Convert SolidDefinitions and PipelineDefinitions to @solid and @pipeline
ClosedPublic

Authored by sashank on Aug 2 2019, 11:56 PM.

Details

Test Plan

unit

Diff Detail

Repository
R1 dagster
Branch
modernize-definitions
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

sashank created this revision.Aug 2 2019, 11:56 PM
sashank added a reviewer: Restricted Project.Aug 5 2019, 4:49 PM
alangenfeld accepted this revision.Aug 5 2019, 5:27 PM
alangenfeld added a subscriber: alangenfeld.

commented out assertion should be brought back, others deleted I would guess. "nits" are up to you.

python_modules/dagster-graphql/dagster_graphql_tests/graphql/setup.py
356–365

why couldnt you switch this one over? is step_metadata_fn not exposed on the decorator?

python_modules/dagster/dagster_tests/core_tests/test_multiple_outputs.py
91–98

nit: can drop input_defs here and let it be inferred

128

?

python_modules/dagster/dagster_tests/core_tests/test_pipeline_errors.py
209–211

nit: can empty the decorator args by renaming the fn to yield_wrong_thing

python_modules/dagster/dagster_tests/core_tests/test_solid_with_config.py
44–47

?

56

?

This revision is now accepted and ready to land.Aug 5 2019, 5:27 PM
sashank updated this revision to Diff 3407.Aug 5 2019, 5:38 PM
sashank marked 5 inline comments as done.

Comments

python_modules/dagster-graphql/dagster_graphql_tests/graphql/setup.py
356–365

yes - should I add it to the decorator?

sashank updated this revision to Diff 3444.Aug 6 2019, 6:20 PM

rebase for tests

alangenfeld added inline comments.Aug 6 2019, 8:22 PM
python_modules/dagster-graphql/dagster_graphql_tests/graphql/setup.py
356–365

ya, i dont think there is any reason to not support it in the decorator