Page MenuHomeElementl

monitor sensor 2/ allow nullable pipeline_name in sensor def

Authored by yuhan on May 12 2021, 4:28 AM.
Referenced Files
Unknown Object (File)
Sat, Mar 25, 4:16 AM
Unknown Object (File)
Tue, Mar 21, 1:24 AM
Unknown Object (File)
Mon, Mar 20, 7:48 PM
Unknown Object (File)
Mon, Mar 20, 4:03 PM
Unknown Object (File)
Mon, Mar 20, 1:15 PM
Unknown Object (File)
Mon, Mar 20, 1:04 PM
Unknown Object (File)
Mon, Mar 20, 10:15 AM
Unknown Object (File)
Mon, Mar 20, 3:22 AM



allow pipeline_name=None in SensorDefinition

Test Plan

unit + local dagit

image.png (650×2 px, 95 KB)

Diff Detail

R1 dagster
Lint Not Applicable
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.May 12 2021, 4:49 AM
Harbormaster failed remote builds in B30456: Diff 37436!

do we want to use name prefixes to distinguish rather than subclassing and having some sort of flag on the external sensor?


the name prefix was the simplest approach. open to alternatives - a flag on SensorDefinition and ExternalSensorData might be cleaner

This revision is now accepted and ready to land.May 14 2021, 8:26 PM

monitor sensor -> pipeline sensor


I suspect that future readers of this code might find it confusing that sensors with _is_pipeline_sensor=True are sensors that do not target a pipeline. It might make sense to give it a name like _is_no_target_sensor? Alternatively, would it make sense to omit it and instead use sensor.pipeline_name is None in the situation where we would otherwise use it?

is_pipeline_sensor -> has_no_target


  • make target nullable (target.pipeline_name is still not nullable)
  • has_no_target -> no_target

I'm a bit skeptical on even having this no_target bool, its effectively just re-encoding state derivable from target being absent. We can fire an experimental warning if neither pipeline_name or job is passed in, but im not sure persisting a bool makes sense

@alangenfeld I was thinking of using no_target as the source of truth so we can error the case of neither pipeline_name or job. But you are right that persisting a bool may be too early. Firing an experimental warning sounds good. Updated.

This revision is now accepted and ready to land.Jun 3 2021, 8:23 PM