Page MenuHomePhabricator

Add Dagster bash solid
ClosedPublic

Authored by natekupp on Aug 2 2019, 6:23 PM.

Details

Reviewers
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:72062de748f6: Add Dagster bash solid
Test Plan

unit

Diff Detail

Repository
R1 dagster
Branch
dagster_bash
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

natekupp created this revision.Aug 2 2019, 6:23 PM
natekupp added a reviewer: Restricted Project.Aug 2 2019, 6:43 PM
alangenfeld requested changes to this revision.Aug 2 2019, 11:52 PM
alangenfeld added a subscriber: alangenfeld.

hm maybe '[bash][{name}]' instead of '[bash solid]'?

python_modules/libraries/dagster-bash/dagster_bash/solids.py
68–72

ehh this seems like it would suck - maybe we should have an enum to control the output logging behavior BUFFER / STREAM / NONE

This revision now requires changes to proceed.Aug 2 2019, 11:52 PM
alangenfeld added inline comments.Aug 2 2019, 11:53 PM
python_modules/libraries/dagster-bash/dagster_bash/solids.py
68–72

should probably do the same thing for spark too - that has the same bad logging output experience right

natekupp added inline comments.Aug 3 2019, 12:45 AM
python_modules/libraries/dagster-bash/dagster_bash/solids.py
68–72

yeah that's a good idea - let me at least do it here, I need to think a little more about how best to handle the Spark case

natekupp updated this revision to Diff 3384.Aug 3 2019, 1:11 AM

comments

alangenfeld accepted this revision.Aug 5 2019, 3:59 PM
alangenfeld added inline comments.
python_modules/libraries/dagster-bash/dagster_bash/solids.py
89

maybe output_logging or something else a little more specific, output_type feels too vague

This revision is now accepted and ready to land.Aug 5 2019, 3:59 PM
natekupp updated this revision to Diff 3401.Aug 5 2019, 5:13 PM

comments

This revision was automatically updated to reflect the committed changes.