Differential D5612

DynamicOutput enforce constraints
ClosedPublic
Actions

Authored by alangenfeld on Dec 15 2020, 4:52 AM.

Tags

None

Referenced Files

	F2278332: D5612.diff
	Sat, Jun 25, 8:55 PM

	Unknown Object (File)
	Sat, Jun 18, 5:56 AM

	Unknown Object (File)
	Fri, Jun 17, 4:33 PM

	Unknown Object (File)
	Mon, Jun 13, 2:18 AM

	Unknown Object (File)
	Mon, May 30, 9:08 AM

	Unknown Object (File)
	Sat, May 28, 3:34 AM

Subscribers

None

Details

Reviewers

• catherinewu
• schrockn

Commits

R1:2f9ca0165b05: DynamicOutput enforce constraints

Summary

ensure that the current limitations of dynamic outputs are enforced with clear error messages.

Test Plan

added tests

Diff Detail

Repository

Lint

Lint Passed

Unit

No Test Coverage

Event Timeline

alangenfeld created this revision.Dec 15 2020, 4:52 AM

Harbormaster completed remote builds in B22854: Diff 27800.Dec 15 2020, 6:03 AM

alangenfeld requested review of this revision.Dec 15 2020, 6:03 AM

cool. plz consider final comments

python_modules/dagster/dagster/core/definitions/dependency.py
458–478	slightly goofy for this to be an inner function given that you aren't capturing anything via closure. (I also thought you were somehow calling it recursively because hard to tell scope with python whitespace ish)
464–466	I'm having trouble parsing this error message
467–474	Not looking forward to explaining the subtleties of this distinction
475	output
475	mild preference for "cannot" rather than "can not"
476	probably should be a 2nd sentence or "as it is downstream"

This revision is now accepted and ready to land.Dec 15 2020, 5:14 PM

feedback

rebase

Harbormaster failed remote builds in B22880: Diff 27831!Dec 15 2020, 5:52 PM

Harbormaster completed remote builds in B22881: Diff 27832.Dec 15 2020, 6:09 PM

alangenfeld added inline comments.Dec 16 2020, 8:09 PM

python_modules/dagster/dagster_tests/core_tests/mapping_tests/test_not_allowed.py
110–117 ↗	(On Diff #27832)	[2]

alangenfeld mentioned this in D5542: DynamicOutput execution support.Dec 16 2020, 8:12 PM

rebase

Harbormaster failed remote builds in B23280: Diff 28318!Dec 21 2020, 10:07 PM

alangenfeld added a child revision: D5721: DynamicOutput DSL map approach.Jan 5 2021, 12:23 AM

alangenfeld added a child revision: D5613: RFC DynamicOutput DSL __iter__ approach.

rebase

Harbormaster failed remote builds in B23821: Diff 28961!Jan 7 2021, 6:15 PM

rebase

Harbormaster completed remote builds in B23837: Diff 28979.Jan 7 2021, 7:58 PM

Closed by commit R1:2f9ca0165b05: DynamicOutput enforce constraints (authored by alangenfeld). · Explain WhyJan 11 2021, 8:25 PM

This revision was automatically updated to reflect the committed changes.

alangenfeld added a commit: R1:2f9ca0165b05: DynamicOutput enforce constraints.

Revision Contents
Changeset List

Path

Size

python_modules/

dagster/

dagster/

core/

definitions/

78 lines

30 lines

i_solid_definition.py

4 lines

3 lines

dagster_tests/

core_tests/

dynamic_tests/

test_not_allowed.py

125 lines

Diff 28961

python_modules/dagster/dagster/core/definitions/dependency.py

Loading...

python_modules/dagster/dagster/core/definitions/graph.py

Loading...

python_modules/dagster/dagster/core/definitions/i_solid_definition.py

Loading...

python_modules/dagster/dagster/core/definitions/solid.py

Loading...

python_modules/dagster/dagster_tests/core_tests/dynamic_tests/test_not_allowed.py

Loading...