Keep recent sensor ticks, regardless of status and add tick garbage collection to sensor loop
ClosedPublic
Actions

Authored by prha on Dec 14 2020, 11:05 PM.

Details

Reviewers

dgibson
schrockn
johann

Commits

R1:e9b6f5f31fd4: Keep recent sensor ticks, regardless of status and add tick garbage collection…

Summary

Useful for looking at the last 20 minutes of ticks for a sensor

Test Plan

Diff Detail

Repository

R1 dagster

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

prha created this revision.Dec 14 2020, 11:05 PM

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 14 2020, 11:19 PM

Harbormaster failed remote builds in B22839: Diff 27782!

update test, rebase

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 14 2020, 11:45 PM

Harbormaster failed remote builds in B22842: Diff 27785!

rebase, update tests

Harbormaster completed remote builds in B22844: Diff 27788.Dec 15 2020, 12:16 AM

prha requested review of this revision.Dec 15 2020, 12:16 AM

Implementation-wise this seems fine - 20 minutes seems kind of conservative though - Not positive about the exact details you have in the UI but are you sure that for whatever we're exposing there people won't want to zoom out beyond 20 minutes? For some visualization/debug purposes I could imagine wanting to go back like 24 hours, and that's only a few thousand rows per sensor, and our indices mean that other than a bit of storage there shouldn't be a huge cost right? But I guess we also have the daemon logs for debugging purposes.

python_modules/dagster/dagster/scheduler/sensor.py
138–139	rather than tempting fate and using the system timezone, create in UTC already using pendulum.now("UTC") so no need to convert to UTC below?
164–169	maybe worth a comment here about why 20 minutes
168	there is also a subtract method :)