New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove jobs no longer referenced in the codebase #38693
base: 2.4-develop
Are you sure you want to change the base?
Remove jobs no longer referenced in the codebase #38693
Conversation
Hi @fredden. Thank you for your contribution! Add the comment under your pull request to deploy test or vanilla Magento instance:
❗ Automated tests can be triggered manually with an appropriate comment:
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
@magento run all tests |
Nice! Good job! @fredden: can you add #38217 to the description of this PR? As this fixes that issue, so it automatically gets closed when this PR is merged. I made the argument in there that maybe we should add the cleanup of unknown jobs to the Recurring setup script that exists in the Cron module, so it only does the cleanup when |
Yes, I'll update this later today. Thanks for highlighting; I hadn't seen that report before now.
I considered this approach too. This would catch most of the scenarios where groups or jobs are removed. There are two (main) reasons why I went with a periodic clean-up instead. If the clean-up is in the The other reason for the periodic clean-up, is that I have seen modules dynamically create jobs, which then never get cleaned up by Magento when that module's configuration changes. |
c84b05d
to
9dea263
Compare
@magento run all tests |
@magento run all tests |
@magento run Database Compare, Functional Tests B2B, Functional Tests CE, Functional Tests EE, Integration Tests, Unit Tests, WebAPI Tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @fredden,
Thanks for the collaboration!
The changes looks good to me but it seems the Integration test has been failing due to changes. I request you to please check the same.
Thanks
@engcom-Hotel thanks for your note. I don't know why I added a circular reference here. I've removed it now. |
@magento run all tests |
@magento run Functional Tests B2B, Functional Tests EE, Functional Tests CE, WebAPI Tests |
bd0a0c6
to
9ebddc6
Compare
I've worked out that this was a typing error. I had intended to add a dependency on |
@magento run all tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failed tests seems flaky to me.
Hi @fredden, Thanks for the collaboration & contribution! ✔️ QA Passed Preconditions:
Steps to reproduce As per the Description. Before: ✖️ After: ✔️ The Jobs for Disabled Modules are removed from the Builds are failed. Hence, moving this PR to Extended Testing. Thanks. |
Description
When upgrading a Magento website over time, some cron jobs are left in the
cron_schedule
table for jobs which no longer exist. This can happen when a module is removed, or when a module removes a cronjob, or when a module changes the name of one of its cronjobs.This pull request adds a daily clean-up task to remove such jobs from the schedule. This means that left-over jobs are pruned safely and the database table does have useless data.
Manual testing scenarios
sitemap_generate
gets scheduled. This belongs to theMagento_Sitemap
module.Magento_Sitemap
module.sitemap_generate
in thecron_schedule
database table.Related issues
Fixes #38217
Contribution checklist