gjcarneiro / yacron

A modern Cron replacement that is Docker-friendly
MIT License
459 stars 38 forks source link

task every45minutes unnecessary second start #16

Open Karmashkin opened 6 years ago

Karmashkin commented 6 years ago

yacron run inside docker alpine 3.6 based python 3.6.1 timezone in the container and on the host is the same.

The report 'onSuccess' comes two times, 45 minutes and 15 minutes after that. in other words, the task runs twice per hour

crontab.yaml

defaults:
  captureStderr: true
  captureStdout: true
  onFailure:
    report:
      mail:
        from: cron@xxxx.xxx
        to: xxx@xxxx.xxx
        smtpHost: xxxxxxxx
        subject: Cron job '{{name}}' {% if success %}completed{% else %}failed{% endif %}
        body: |
          command: {{command}}
          shell: {{shell}}
          environment: {{environment}}
          STDOUT: {{stdout}}
          STDERR: {{stderr}}
          exit code: {{exit_code}}

jobs:
- name: 45minutes
  command: /usr/bin/php5 /xxx/xxx/xxx.php
#  schedule: "*/45 * * * *"
  schedule:
    minute: "*/45"
  onSuccess:
    report:
      mail:
        from: cron@xxxx.xxx
        to: xxx@xxxx.xxx
        smtpHost: xxxxxxxx
gjcarneiro commented 6 years ago

This is true, because that's what the Python crontab module decides:

>>> CronTab("*/45 * * * *").test(datetime(year=2017, month=12, day=5, hour=15, minute=0))
True
>>> CronTab("*/45 * * * *").test(datetime(year=2017, month=12, day=5, hour=15, minute=45))
True

To be honest, I don't think this is a bug. If you want it to run only once an hour, at minute 45, you can specify minute: 45 instead of minute: */45.

In general, specifying "every X minutes" where X is not an exact divisor of an hour, is just confusing.

If you expect that */45 runs at e.g. 15h00, 15h45, 16h30, 17h15, etc., then no, this is too hard to code, and makes yacron not handle restarts well: it would have to store the state of when it last ran a job elsewhere. I honestly don't want to down that road.

I think this is something yacron can try to catch as configuration error, though.

Karmashkin commented 6 years ago

it can become your killer features :) read Google, I agree it's not a bug