allow pipeline_name=None in SensorDefinition
Details
Details
unit + local dagit
Diff Detail
Diff Detail
- Repository
- R1 dagster
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| python_modules/dagster/dagster/core/definitions/repository.py | ||
|---|---|---|
| 554 | do we want to use name prefixes to distinguish rather than subclassing and having some sort of flag on the external sensor? | |
| python_modules/dagster/dagster/core/definitions/repository.py | ||
|---|---|---|
| 554 | the name prefix was the simplest approach. open to alternatives - a flag on SensorDefinition and ExternalSensorData might be cleaner | |
| python_modules/dagster/dagster/core/definitions/sensor.py | ||
|---|---|---|
| 169 | 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? | |
Comment Actions
rebase
- make target nullable (target.pipeline_name is still not nullable)
- has_no_target -> no_target
Comment Actions
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
Comment Actions
@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.