I'm having trouble with sensors and `SkipReason`s ...
# ask-community
a
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?
c
Hi Adam, what version of dagster are you running? As of the latest version (1.0.13), running a sensor with something like:
Copy code
@multi_asset_sensor(
    ...
)
def my_multi_asset_sensor(context):    
    return SkipReason("my message")
should display with the message passed in the skip reason
a
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:
Copy code
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:
Copy code
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
perhaps relevant: this sensor is embedded in a factory function. Not sure why it would break this though
c
thanks for the info, I'm looking into this now
a
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
stack trace from changing
yield RunRequest
to `return RunRequest`:
Copy code
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(
c
Ah, since python 3.3 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
a
I get the same exception above if I change the
SkipReason
to yield
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?
c
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
a
When you say generator fns, are you referring to a python generator? trying to figure out where that is coming into play.
c
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
a
since it is `yield`ing the result of the wrapped function?
(my sensor isn't a generator that I'm aware of, so just trying to put the pieces together)
c
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
a
yeah, but when it contains two
return
s, I still get that error and that function shouldn't be a generator
but that is `yield`ed by the decorator when it wraps the function, so that might do it?
your fix works with
return
for both the
RunRequest
and
SkipReason
btw, thanks! This is what I was expecting:
Copy code
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
c
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