Page MenuHomeElementl

[RFC] remove PIPELINE_INIT_FAILURE event
ClosedPublic

Authored by yuhan on Jun 3 2021, 7:05 PM.
Tags
None
Referenced Files
F2432019: D8231.diff
Sun, Aug 14, 6:12 AM
Unknown Object (File)
Sat, Aug 13, 12:10 AM
Unknown Object (File)
Fri, Aug 12, 4:15 AM
Unknown Object (File)
Fri, Aug 12, 4:13 AM
Unknown Object (File)
Fri, Aug 12, 4:12 AM
Unknown Object (File)
Fri, Aug 12, 4:11 AM
Unknown Object (File)
Fri, Aug 12, 4:10 AM
Unknown Object (File)
Thu, Aug 4, 6:41 PM
Subscribers
None

Details

Summary

squash PIPELINE_INIT_FAILURE into PIPELINE_FAILURE

Test Plan
  • backcompat test
  • dagit before:
    Screen Shot 2021-06-04 at 9.52.04 PM.png (3×2 px, 1 MB)
    after:
    Screen Shot 2021-06-04 at 9.53.43 PM.png (3×2 px, 1 MB)

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 3 2021, 7:26 PM
Harbormaster failed remote builds in B31622: Diff 38932!
Harbormaster returned this revision to the author for changes because remote builds failed.Jun 3 2021, 8:09 PM
Harbormaster failed remote builds in B31634: Diff 38948!

pipeline_context -> pipeline_context_or_name so mypy is happy and code isn't too ugly

think this looks about right to me

python_modules/dagster/dagster/core/events/__init__.py
1421–1434

dont forget to add a tombstone

yuhan marked an inline comment as done.

+tombstone

ill leave this up a little longer to allow others to chime in

seems likely well land this though, can you add a compat test that has a pipeline init failure in it ? https://dagster.phacility.com/source/dagster/browse/master/python_modules/dagster/dagster_tests/general_tests/compat_tests

maybe also a before and after dagit screenshot if you are feeling generous

python_modules/dagster/dagster/core/events/__init__.py
1422

FOR WHOM THE BELL TOLLS

i agree with alex and think this is just great

+test
bring PipelineInitFailureData so old runs wont crash in dagit

welldone

python_modules/dagster/dagster/core/events/__init__.py
770–773

can we just use the context_msg like above for the init failure case instead of encoding these assumptions about use - i think its better to dupe at call sites then to have this message show up unexpectedly in some future case

This revision is now accepted and ready to land.Jun 8 2021, 12:51 AM
yuhan edited the test plan for this revision. (Show Details)

up

This revision was automatically updated to reflect the committed changes.