Page MenuHomeElementl

slack pipeline_failure_sensor
ClosedPublic

Authored by yuhan on Jun 24 2021, 6:40 AM.

Details

Summary

make_slack_on_pipeline_failure_sensor
^ api pair with slack_on_failure (here)

doc:

image.png (2×1 px, 431 KB)

slack_on_pipeline_failure = make_slack_on_pipeline_failure_sensor(
    "#yuhan-test",
    os.getenv("SLACK_DAGSTER_ETL_BOT_TOKEN"),
    dagit_base_url="http://localhost:3000",

@repository
def my_repo():
    return [my_pipeline, slack_on_pipeline_failure]

slack msg:

Screen Shot 2021-06-23 at 11.37.24 PM.png (304×1 px, 79 KB)

Test Plan

unit test for def
toy sensor worked in dagit

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

yuhan edited the summary of this revision. (Show Details)
Harbormaster returned this revision to the author for changes because remote builds failed.Jun 24 2021, 7:05 AM
Harbormaster failed remote builds in B32571: Diff 40127!
yuhan edited the summary of this revision. (Show Details)

up

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 24 2021, 7:59 AM
Harbormaster failed remote builds in B32572: Diff 40128!
yuhan requested review of this revision.Jun 24 2021, 6:28 PM

move slack client creation to def time

remove from toy - will addd the example to docs snippet later

This looks great - just had a few small comments.

python_modules/libraries/dagster-slack/dagster_slack/sensors.py
14

It would be helpful to include the mode name as well if it's not default.

25

This might be clearer as slack_token_env_var. Also, I haven't thought about this at all, but why not pass the slack token directly instead of an env var?

30

step -> pipeline and hook -> sensor

38

extra_message_fn vs. message_fn

39

"and outputs"

This revision is now accepted and ready to land.Jun 24 2021, 8:57 PM
python_modules/libraries/dagster-slack/dagster_slack/sensors.py
14

i wrote this yday and was just thinking of the new world without mode. will add

25

the slack token is usually considered as secret so passing it in as plain text isn't perfect. if it's a resource, we could do StringSource to avoid the awkwardness here.

on a second thought and looked at the our existing dagster-slack apis, i think it's fine to take token value directly and just have an example to do token=os.getenv("MY_TOKEN"). will change to a required token arg then

38

oops i thought i updated it.

yuhan marked 5 inline comments as done.

feedback

This revision was automatically updated to reflect the committed changes.