Page MenuHomeElementl
Differential D9053

dagstermill - fix: yield_result temp path includes solid name
ClosedPublic
Actions

Authored by • yuhan on Jul 26 2021, 6:54 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 21, 5:23 AM
Unknown Object (File)
Sun, Mar 19, 12:33 AM
Unknown Object (File)
Tue, Mar 14, 8:53 AM
Unknown Object (File)
Tue, Mar 14, 7:49 AM
Unknown Object (File)
Tue, Mar 14, 7:49 AM
Unknown Object (File)
Tue, Mar 14, 7:49 AM
Unknown Object (File)
Mon, Mar 13, 2:54 AM
Unknown Object (File)
Fri, Mar 10, 1:05 PM
View All 85 Files
Subscribers
None

Details

Reviewers
alangenfeld
• max
Commits
R1:ad5abf66f0b2: dagstermill - fix: yield_result temp path includes solid name
Summary

fix a bug where yield_result wrote output value to the same file path if output names are the same for different solids.
it caused random serdes error in multiproc bc write and read could happen the same time for different objects to the same file

Test Plan

unit - fan_in_notebook_pipeline is the repro

before the fix, both solids yield "world" because one overwrites the other:

Screen Shot 2021-07-25 at 11.37.55 PM.png (776×2 px, 351 KB)

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• yuhan created this revision.Jul 26 2021, 6:54 AM
• yuhan updated this revision to Diff 42250.Jul 26 2021, 6:56 AM

the real fix

• yuhan edited the test plan for this revision. (Show Details)Jul 26 2021, 6:57 AM
• yuhan added reviewers: alangenfeld, • max.
• yuhan edited the test plan for this revision. (Show Details)
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 26 2021, 7:37 AM
Harbormaster failed remote builds in B34181: Diff 42250!
Harbormaster failed remote builds in B34180: Diff 42249!
• yuhan updated this revision to Diff 42276.Jul 26 2021, 5:35 PM

up

• yuhan updated this revision to Diff 42280.Jul 26 2021, 5:50 PM

up

Harbormaster completed remote builds in B34203: Diff 42280.Jul 26 2021, 6:16 PM
• yuhan requested review of this revision.Jul 26 2021, 6:16 PM
Harbormaster completed remote builds in B34200: Diff 42276.Jul 26 2021, 6:20 PM
alangenfeld accepted this revision.Jul 27 2021, 4:13 PM

looking forward to a more thorough revamp of this stuff

This revision is now accepted and ready to land.Jul 27 2021, 4:13 PM
Closed by commit R1:ad5abf66f0b2: dagstermill - fix: yield_result temp path includes solid name (authored by • yuhan). · Explain WhyJul 27 2021, 5:35 PM
This revision was automatically updated to reflect the committed changes.
• yuhan added a commit: R1:ad5abf66f0b2: dagstermill - fix: yield_result temp path includes solid name.
• yuhan mentioned this in D9110: dagstermill - yield_result output path account for aliased solids and composite solids.Jul 27 2021, 11:49 PM
• yuhan mentioned this in R1:f1c0a4f53e57: dagstermill - yield_result output path account for aliased solids and composite….Jul 28 2021, 11:23 PM

Revision Contents
Changeset List

PathSize
python_modules/
libraries/
dagstermill/
dagstermill/
examples/
notebooks/
47 lines
82 lines
2 lines
5 lines
1 line
dagstermill_tests/
18 lines

Diff 42380

View Options

python_modules/libraries/dagstermill/dagstermill/examples/notebooks/yield_something.ipynb

Loading...
View Options

python_modules/libraries/dagstermill/dagstermill/examples/repository.py

Loading...
View Options

python_modules/libraries/dagstermill/dagstermill/examples/workspace.yaml

Loading...
View Options

python_modules/libraries/dagstermill/dagstermill/manager.py

Loading...
View Options

python_modules/libraries/dagstermill/dagstermill/solids.py

Loading...
View Options

python_modules/libraries/dagstermill/dagstermill_tests/test_solids.py

Loading...