Page MenuHomeElementl

[RFC] remove PIPELINE_INIT_FAILURE event
ClosedPublic

Authored by yuhan on Jun 3 2021, 7:05 PM.

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!

-PipelineInitFailureData

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

yuhan requested review of this revision.Jun 3 2021, 9:15 PM

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.