Page MenuHomeElementl

[crag] Job description
ClosedPublic

Authored by alangenfeld on Jul 19 2021, 2:49 PM.

Details

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.Jul 19 2021, 3:30 PM
Harbormaster failed remote builds in B33843: Diff 41799!
alangenfeld added inline comments.
python_modules/dagster/dagster_tests/core_tests/graph_tests/test_graph.py
249–251

^ looking for feedback here

Do you have thoughts on what the dagit experience should be like? I.e. where does graph description vs. job description show up?

Im not sure. I think the graph page having the graph desc is clear.

I could see the job page showing both (hiding when None) which would imply not inheriting

I could see the job page just showing just job desc which would support inheriting.

Should we start here without and change it to add later based on dagit iteration?

python_modules/dagster/dagster_tests/core_tests/graph_tests/test_graph.py
251

My intuition would be that we would default to the graph's desc, and the desc arg on to_job would act as an override almost.

python_modules/dagster/dagster_tests/core_tests/graph_tests/test_graph.py
251

that behavior does align with name

I think its a reasonable approach to land this and change the defaulting-from-graph behavior later if we decide its the right thing to do

This revision is now accepted and ready to land.Jul 22 2021, 3:47 PM
This revision was automatically updated to reflect the committed changes.