- Issue created by @O'Briat
- First commit to issue fork.
- Merge request !2Issue #3379107: Simple cron should log failed manual executions. → (Open) created by mukhtarm
- Status changed to Needs review
over 1 year ago 12:54pm 24 August 2023 - 🇫🇷France O'Briat Nantes
We push simultaneously :)
Anyway the MR is ready to be RTBC
- Status changed to RTBC
11 months ago 8:53am 4 January 2024 - 🇫🇷France O'Briat Nantes
I mark this issue as RTBTC as it run smoothly on production websites for 4 months
- Status changed to Needs work
11 months ago 10:05am 4 January 2024 - 🇫🇷France O'Briat Nantes
I need more testing, this patch add too much logging,
CronJob::runalready log successful and failed job - 🇫🇷France O'Briat Nantes
O'Briat → changed the visibility of the branch 3379107-simple-cron-should to hidden.
- 🇮🇳India abhaisasidharan
Additionally, if the Simple Cron job fails, it does not update the last run. Let me explain why this could be a potential issue.
In our project, we have a Simple Cron job that loops through a set of emails and send a notification (based on a condition: If date is 14 days from today).
This cron should execute once a day if all goes well. If the cron job fails, there is no long showing that it failed, and the last run is not updated.
The core-cron is set to run every 20 minutes. On the next core-cron run, it will check for last run time for the simple cron job that failed and will not find it and execute it again until it fails again. If it failed after sending 20 emails, and there are still 30 more to go. The first 20 emails will get repeated emails every time the core-cron runs (20 minutes).Solution:
The simple cron job, should update last run time even if it fails. The responsibility of making sure it doesn't fail belongs to the developer. - 🇬🇧United Kingdom scott_euser
Fine to create as a separate issue I think. This one is okay, but coding standards should be followed (which currently are not). Updated issue summary to reflect that.
- 🇫🇷France O'Briat Nantes
Phpcs fixed, I also added two minors fixes on
CronTest.php - 🇬🇧United Kingdom scott_euser
Thanks! CronTest.php phpcs seems like its out of scope, I don't think phpcs should be applied to existing code as part of this issue, that is getting resolved in other issues.
- 🇫🇷France O'Briat Nantes
Yes, I know but with just two carriage returns all the module now complies with the Drupal standards. If you think it's not needed, I'll revert it.
- 🇬🇧United Kingdom scott_euser
Nah all good, does no harm, ignore me :) Anyways I put in an application to take over maintenance since the maintainer has been quiet for a long time now. If that does go ahead I'll get a bunch of issues merged in + test coverage running again once 📌 Automated Drupal 11 compatibility fixes for simple_cron Needs review is in