I m having trouble with sensors and `SkipReason`s I return o

Question

I m having trouble with sensors and `SkipReason`s I return one but I still see `Sensor function returned an empty result` rather than the message passed in the skip reason Am I missing something This is what is shown in the examples but just doesn t seem to work in reality Note that I ve mostly been using multi asset sensors so perhaps this works with regular asset sensors but not multi asset

claire · Answer

Hi Adam what version of dagster are you running As of the latest version 1 0 13 running a sensor with something like ``` multi asset sensor def my multi asset sensor context return SkipReason my message ``` should display with the message passed in the skip reason

Adam Bloom · Answer

I m running 1 0 13 And that is exactly the sort of thing I ve tested and does not work here s a super simple example ``` def dbt test docs sensor context asset events = context latest materialization records by key if all asset events values context advance all cursors yield RunRequest else return SkipReason Waiting for new dbt assets ``` logs ```2022 10 17 16 39 06 0600 dagster daemon SensorDaemon INFO Checking for new runs for sensor dbt test and doc events 2022 10 17 16 39 06 0600 dagster daemon SensorDaemon INFO Sensor dbt test and doc events skipped Sensor function returned an empty result```

Adam Bloom · Answer

perhaps relevant this sensor is embedded in a factory function Not sure why it would break this though

claire · Answer

thanks for the info I m looking into this now

Adam Bloom · Answer

There s some other interesting things that happen when you change yield vs return I think dagster daemon doesn t start when I had the `RunRequest` return instead of yield I can grab a log for that in a moment in the middle of testing something else

Adam Bloom · Answer

stack trace from changing `yield RunRequest` to `return RunRequest` ```2022 10 17 16 47 37 0600 dagster daemon SensorDaemon ERROR Sensor daemon caught an error for sensor dbt test and doc events dagster core errors DagsterInvalidDefinitionError Asset materializations have been handled in this sensor but the cursor was not updated This means the same materialization events will be handled in the next sensor tick Use context advance cursor or context advance all cursors to update the cursor Stack Trace File Users adambloom Library Caches pypoetry virtualenvs data pipeline Z0nElMr1 py3 9 lib python3 9 site packages dagster grpc impl py line 320 in get external sensor execution return sensor def evaluate tick sensor context File Users adambloom Library Caches pypoetry virtualenvs data pipeline Z0nElMr1 py3 9 lib python3 9 site packages dagster core definitions sensor definition py line 1268 in evaluate tick result = list self evaluation fn context File Users adambloom Library Caches pypoetry virtualenvs data pipeline Z0nElMr1 py3 9 lib python3 9 site packages dagster core definitions sensor definition py line 1421 in wrapped fn for item in result File Users adambloom Library Caches pypoetry virtualenvs data pipeline Z0nElMr1 py3 9 lib python3 9 site packages dagster core definitions sensor definition py line 1797 in fn raise DagsterInvalidDefinitionError ```

claire · Answer

Ah since python 3 3 <https stackoverflow com questions 16780002 return in generator together with yield|a return value in a generator> is now treated as a `raise StopIteration ` call So because your sensor contains both a yield and a return statement the skip reason is ignored as it is passed to the `StopIteration` exception Instead you could make the function return all values instead of yielding for example by returning a list of run requests Or you can change the `return SkipReason` to `yield SkipReason`

Adam Bloom · Answer

I get the same exception above if I change the `SkipReason` to yield

Adam Bloom · Answer

also related issue <https github com dagster io dagster issues 4429>

Adam Bloom · Answer

so I clearly need to change to return but then why am I getting that exception about triggering asset materializations when the sensor is first loaded

claire · Answer

Yeah the `Asset materializations have been handled in this sensor but the cursor was not updated ` error is raised erroneously for generator functions I can fix this for the next release

Adam Bloom · Answer

When you say generator fns are you referring to a python generator trying to figure out where that is coming into play

claire · Answer

<https github com dagster io dagster pull 10059> Here s a fix for this issue It s occurring because the multi asset sensor mistakenly raises the error if a skip reason is yielded within a generator multi asset sensor function

Adam Bloom · Answer

ah and the wrapper for `multi asset sensor` is the generator right <https github com dagster io dagster blob master python modules dagster dagster core definitions decorators sensor decorator py L247 L248>

Adam Bloom · Answer

since it is `yield`ing the result of the wrapped function

Adam Bloom · Answer

my sensor isn t a generator that I m aware of so just trying to put the pieces together

claire · Answer

ah no I mean that the generator function is the decorated function In this case I m referring to `dbt test docs sensor` as the generator function as it contains a `yield` statement

Adam Bloom · Answer

yeah but when it contains two `return` s I still get that error and that function shouldn t be a generator

Adam Bloom · Answer

but that is `yield`ed by the decorator when it wraps the function so that might do it

Adam Bloom · Answer

your fix works with `return` for both the `RunRequest` and `SkipReason` btw thanks This is what I was expecting ```2022 10 17 17 21 23 0600 dagster daemon SensorDaemon INFO Checking for new runs for sensor dbt test and doc events 2022 10 17 17 21 23 0600 dagster daemon SensorDaemon INFO Sensor dbt test and doc events skipped Waiting for new dbt assets```

claire · Answer

Ah yeah the wrapped function that you linked to is the generator that causes the error that you were seeing Nice glad that this fix resolves the error It should make it out in the `1 0 14` release