jfrag1 commented on code in PR #25239:
URL: https://github.com/apache/superset/pull/25239#discussion_r1324879798
##########
superset/tasks/cron_util.py:
##########
@@ -39,9 +39,9 @@ def cron_schedule_window(cron: str, timezone: str) ->
Iterator[datetime]:
logger.warning("Timezone %s was invalid. Falling back to 'UTC'",
timezone)
utc = pytz_timezone("UTC")
# convert the current time to the user's local time for comparison
- time_now = time_now.astimezone(tz)
- start_at = time_now - timedelta(seconds=1)
- stop_at = time_now + timedelta(seconds=window_size)
+ time_now = triggered_at.astimezone(tz)
+ start_at = time_now - timedelta(seconds=window_size / 2)
+ stop_at = time_now + timedelta(seconds=window_size / 2)
Review Comment:
I don't think the stop_at will prevent anything we want to run from running.
Say the job is triggered at 12:00, but run at 12:02, and the cron says the job
should run at 12:00. `start_at` will be ~11:59:30, and `stop_at` will be
~12:00:30, so it will correctly find the 12:00 schedule since it's in the
window.
Also I don't believe cron supports second level granularity, at least not
that I've seen in Superset. IMO for Superset's use cases, getting the
reliability right is much more important than supporting that level of
granularity
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
