Page MenuHomeElementl

[crag] sensor & schedule direct target
ClosedPublic

Authored by alangenfeld on May 18 2021, 4:42 PM.

Details

Summary

first pass - more validation could be done but i think this gets things working so that we can play

depends on D7962

Test Plan

added tests

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

an open question since we have been discussing targets , will we want to support target and targets` arguments on these decorators / definitions? Maybe just the decorators?

If yes I don't think there is sequencing problem, if no we may want to setup targets earlier than later

python_modules/dagster/dagster_tests/core_tests/definitions_tests/test_repository_definition.py
211–217

I can switch these out to graph.with_resources too

whoops, thought I hit submit yesterday:

My reaction is yes, we do want to support it on both.

Sweet

python_modules/dagster/dagster/core/definitions/decorators/schedule.py
44

Do we intend to make this available to users? If not, then worth signaling that somehow? E.g. via experimental warning or underscore or documentation?

an open question since we have been discussing targets , will we want to support target and targets` arguments on these decorators / definitions? Maybe just the decorators?

My gut is to at least start by not supporting targets` arguments on these decorators / definitions. Because, as people have brought up, what if we want to add resources to sensors and schedules and it becomes confusing whether the resources apply to the target or to the sensor/schedule?

python_modules/dagster/dagster/core/definitions/decorators/schedule.py
44

Or, to contextualize the question above, is the idea that we'd merge this diff now? Or that it's a building block someone could use to experiment with cragly changes?

python_modules/dagster/dagster/core/definitions/decorators/schedule.py
44

I think its pretty safe to land - with the missing piece being how to communicate this to users who may stumble across it. Can do experimental warning/docs.

This revision is now accepted and ready to land.Fri, May 28, 5:39 PM
python_modules/dagster/dagster/core/definitions/decorators/sensor.py
14

[1]

This revision was automatically updated to reflect the committed changes.