Page MenuHomeElementl

single asset sensor definition
ClosedPublic

Authored by prha on Jun 15 2021, 8:21 PM.

Details

Summary

Create a query-able asset sensor definition to fetch upstream assets nodes from pipeline
definitions.

Test Plan

bk, migrated toy asset sensor

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 15 2021, 8:40 PM
Harbormaster failed remote builds in B32136: Diff 39586!
Harbormaster returned this revision to the author for changes because remote builds failed.Jun 15 2021, 9:43 PM
Harbormaster failed remote builds in B32141: Diff 39594!
prha requested review of this revision.Jun 15 2021, 10:11 PM

^^ spurious

Overall, I like this direction, but I do think there are many cases where you might want to depend on multiple asset keys at once (the story_recommender pipeline is one example). This does add bits of complexity basically everywhere (need a fancier cursor object, and it's not obvious what exactly should be passed into the user's sensor function when just a single one of the many asset dependencies has been updated), but I do think that people will look for this functionality pretty quickly, and it would be nice to have a good story for them..

Yes, we'll have to provide a way to handle multi-asset sensors, but I think the guarantees provided by the single asset API are valuable enough that this should be pulled out separately.

We can provide an @multi_asset_sensor or something like that that has fewer guarantees around source run attribution.

Overall I am on board with this. It still needs docs etc. and I left a comment on one of the details of the API.

python_modules/dagster/dagster/core/definitions/sensor.py
116

Would be good to add type annotations.

119

Thoughts on making source_run_ids an attribute of RunRequest? When I see "add_source_run_id" here, I'm not sure what entity it's getting added to.

queue management

This revision now requires changes to proceed.Jun 28 2021, 4:07 PM

update documentation, remove source ids, for simplicity

sandyryza added inline comments.
docs/content/concepts/partitions-schedules-sensors/sensors.mdx
319

probably better to just JSON-encode than use a custom format (I think I originated this code, but regret it)

This revision is now accepted and ready to land.Jun 29 2021, 4:11 PM

rebase, using new event records API

prha retitled this revision from RFC: single asset sensor definition to single asset sensor definition.Jul 3 2021, 12:52 AM
prha edited the summary of this revision. (Show Details)
prha edited the summary of this revision. (Show Details)

fix bad merge

This revision was automatically updated to reflect the committed changes.