From 296458ece33cbe60f496a76aa51fbb29faee2fad Mon Sep 17 00:00:00 2001 From: Fabio Manganiello Date: Tue, 9 Mar 2021 00:18:33 +0100 Subject: [PATCH] Cron expressions should follow the machine local time, not UTC [closes #173] --- .gitignore | 1 - CHANGELOG.md | 7 ++++++ platypush/cron/scheduler.py | 17 +++++-------- tests/etc/scripts/__init__.py | 1 + tests/etc/scripts/test_cron.py | 26 +++++++++++++++++++ tests/test_cron.py | 46 ++++++++++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 tests/etc/scripts/__init__.py create mode 100644 tests/etc/scripts/test_cron.py create mode 100644 tests/test_cron.py diff --git a/.gitignore b/.gitignore index b2138637d..a8b302d7c 100644 --- a/.gitignore +++ b/.gitignore @@ -18,5 +18,4 @@ platypush/notebooks platypush/requests /http-client.env.json /platypush/backend/http/static/css/dist -/tests/etc/scripts /tests/etc/dashboards diff --git a/CHANGELOG.md b/CHANGELOG.md index 1633341fd..e1705886e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ All notable changes to this project will be documented in this file. Given the high speed of development in the first phase, changes are being reported only starting from v0.20.2. +## [unreleased] + +### Fixed + +- Cron expressions should adhere to the UNIX cronjob standard and use the machine local time, + not UTC, as a reference (closes [#173](https://git.platypush.tech/platypush/platypush/-/issues/173)). + ## [0.20.4] - 2021-03-08 ### Added diff --git a/platypush/cron/scheduler.py b/platypush/cron/scheduler.py index c3a49b390..9c7cd04cf 100644 --- a/platypush/cron/scheduler.py +++ b/platypush/cron/scheduler.py @@ -1,9 +1,10 @@ +import datetime import enum import logging import threading -import time import croniter +from dateutil.tz import gettz from platypush.procedure import Procedure from platypush.utils import is_functional_cron @@ -56,16 +57,10 @@ class Cronjob(threading.Thread): self.state = CronjobState.ERROR def wait(self): - now = int(time.time()) + now = datetime.datetime.now().replace(tzinfo=gettz()) cron = croniter.croniter(self.cron_expression, now) - next_run = int(cron.get_next()) - self._should_stop.wait(next_run - now) - - def should_run(self): - now = int(time.time()) - cron = croniter.croniter(self.cron_expression, now) - next_run = int(cron.get_next()) - return now == next_run + next_run = cron.get_next() + self._should_stop.wait(next_run - now.timestamp()) def stop(self): self._should_stop.set() @@ -117,7 +112,7 @@ class CronScheduler(threading.Thread): if job.state == CronjobState.IDLE: job.start() - time.sleep(0.5) + self._should_stop.wait(timeout=0.5) logger.info('Terminating cron scheduler') diff --git a/tests/etc/scripts/__init__.py b/tests/etc/scripts/__init__.py new file mode 100644 index 000000000..f4f707fd4 --- /dev/null +++ b/tests/etc/scripts/__init__.py @@ -0,0 +1 @@ +# Auto-generated __init__.py - do not remove diff --git a/tests/etc/scripts/test_cron.py b/tests/etc/scripts/test_cron.py new file mode 100644 index 000000000..6618dcb3f --- /dev/null +++ b/tests/etc/scripts/test_cron.py @@ -0,0 +1,26 @@ +import datetime + +from platypush.cron import cron + +from tests.test_cron import tmp_files, tmp_files_ready, \ + test_timeout, expected_cron_file_content + +# Prepare a cronjob that should start test_timeout/2 seconds from the application start +cron_time = datetime.datetime.now() + datetime.timedelta(seconds=test_timeout/2) +cron_expr = '{min} {hour} {day} {month} * {sec}'.format( + min=cron_time.minute, hour=cron_time.hour, day=cron_time.day, + month=cron_time.month, sec=cron_time.second) + + +@cron(cron_expr) +def cron_test(**_): + """ + Simple cronjob that awaits for ``../test_cron.py`` to be ready and writes the expected + content to the monitored temporary file. + """ + files_ready = tmp_files_ready.wait(timeout=test_timeout) + assert files_ready, \ + 'The test did not prepare the temporary files within {} seconds'.format(test_timeout) + + with open(tmp_files[0], 'w') as f: + f.write(expected_cron_file_content) diff --git a/tests/test_cron.py b/tests/test_cron.py new file mode 100644 index 000000000..30620d991 --- /dev/null +++ b/tests/test_cron.py @@ -0,0 +1,46 @@ +import os +import pytest +import tempfile +import threading +import time + +tmp_files = [] +tmp_files_ready = threading.Event() +test_timeout = 10 +expected_cron_file_content = 'The cronjob ran successfully!' + + +@pytest.fixture(scope='module', autouse=True) +def tmp_file(*_): + tmp_file = tempfile.NamedTemporaryFile(prefix='platypush-test-cron-', + suffix='.txt', delete=False) + tmp_files.append(tmp_file.name) + tmp_files_ready.set() + yield tmp_file.name + + if os.path.isfile(tmp_files[0]): + os.unlink(tmp_files[0]) + + +def test_cron_execution(tmp_file): + """ + Test that the cronjob in ``../etc/scripts/test_cron.py`` runs successfully. + """ + actual_cron_file_content = None + test_start = time.time() + + while actual_cron_file_content != expected_cron_file_content and \ + time.time() - test_start < test_timeout: + with open(tmp_file, 'r') as f: + actual_cron_file_content = f.read() + time.sleep(0.5) + + assert actual_cron_file_content == expected_cron_file_content, \ + 'cron_test failed to run within {} seconds'.format(test_timeout) + + +if __name__ == '__main__': + pytest.main() + + +# vim:sw=4:ts=4:et: