Page MenuHomeElementl

[RFC] pipeline contains a graph
ClosedPublic

Authored by alangenfeld on Feb 23 2021, 11:26 PM.

Details

Summary

Change PipelineDefinition from being a GraphDefinition to *containing* a GraphDefinition.

I tried to avoid breaking changes here, notably

  • only changing PipelineDefinition constructor by adding graph_def
  • adding proxy methods for graph properties. Since i highly doubt users hit any of these - I could also migrate the callsites to go through .graph
Test Plan

bk

Diff Detail

Repository
R1 dagster
Branch
graph-in-pipe-2 (branched from master)
Lint
Lint Warnings
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 23 2021, 11:43 PM
Harbormaster failed remote builds in B26352: Diff 32207!
Harbormaster returned this revision to the author for changes because remote builds failed.Feb 24 2021, 12:20 AM
Harbormaster failed remote builds in B26360: Diff 32216!
Harbormaster returned this revision to the author for changes because remote builds failed.Feb 24 2021, 4:09 PM
Harbormaster failed remote builds in B26378: Diff 32233!
alangenfeld retitled this revision from pipeline contains a graph to [RFC] pipeline contains a graph.Feb 24 2021, 6:39 PM
alangenfeld added reviewers: schrockn, sandyryza.
This revision now requires changes to proceed.Mar 17 2021, 6:41 PM

depends on what path we want to take with Graph but seems like some futures involve this change or something similar so keeping it rebased

store pipeline level tags seperate from graph level, same for name

This will enable a lot of stuff. Thanks !
Macro blastoise:

This revision is now accepted and ready to land.Jul 1 2021, 10:28 PM
This revision was automatically updated to reflect the committed changes.