Skip to content

Conversation

Massi-X
Copy link

@Massi-X Massi-X commented Dec 1, 2023

This is a repost of the same PR made in Jira.

Following a problem I had with certman failing to renew certificates because its crontab was not executed, I noticed that certman is still using the long deprecated Cron class. This PR fixes the behavior by switching to the standard Job class.

Cron is long deprecated
No need to do things here anymore. Initialization is done inside the BMO class.
Copy link

@jfinstrom jfinstrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no job class so this does nothing

@Massi-X
Copy link
Author

Massi-X commented Dec 5, 2023

Yep you are right. Copied over from Jira by memory... I will have a look later and update the PR

@Massi-X
Copy link
Author

Massi-X commented Dec 5, 2023

And here we go, added missing class

Copy link

@jfinstrom jfinstrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks correct now

@Massi-X
Copy link
Author

Massi-X commented Jan 30, 2024

Hi any update on this? Mi system again lost the crontab (don't know why...). Switching to Job will fix the issue

@kguptasangoma kguptasangoma self-requested a review February 12, 2024 04:05
@kguptasangoma
Copy link
Member

Hi @Massi-X

"update" certificates involves the network call and if that's gets delayed due to any network issue then subsequent processes/tasks in the jobs queue will get delayed.
Right now "fwconsole jobs" works in sequentially and if one job gets failed none of the other jobs will gets executed , you can say this is current limitation of the "fwconsole jobs" , this is the main reason of keeping this "update" in the cron jobs to avoid affecting other tasks in the jobs queue.

Did you tried to check why cron job is getting removed automatically from your linux server? may be /var/log/cron or /var/log/message can give you some idea.

@Massi-X
Copy link
Author

Massi-X commented Feb 12, 2024

Hi Kapil, that's a good point. I will have a look or I could also implement a parallel Job system, shouldn't be to hard

@kguptasangoma
Copy link
Member

This is the main reason of keeping this in linux cron and not adding to the "fwconsole jobs" @Massi-X .

We have plan to revisit the "fwconsole jobs" for FreePBX 17 so may be worth to take a look in 17 where we have all the updates libs.

@jfinstrom
Copy link

jfinstrom commented Feb 12, 2024

This is the main reason of keeping this in linux cron and not adding to the "fwconsole jobs" @Massi-X .

We have plan to revisit the "fwconsole jobs" for FreePBX 17 so may be worth to take a look in 17 where we have all the updates libs.

I am not sure why this is a "future" item as the fix to that is simply adding a try catch. This is out of scope on this PR but

private function runJobs($jobs = []) {
    // ... (Existing code) ...

    foreach($jobs as $config) {

        // ... (Existing code within the loop) ...

        try { 
           // ... (Job execution logic) ... 
        } catch (\Exception $e) {
            $msg = "<error> ". sprintf(_("Error in the task %s Exception = %s"),$config['job'],$e->getMessage())." </error>";
            $this->writelog($msg);

            // Optionally, you might want to add specific failure handling or 
            // retry logic for the individual job here.
        }
    }

    // ... (Rest of the function) ...
}

@Massi-X
Copy link
Author

Massi-X commented Feb 12, 2024

@jfinstrom yeah I did this for one of my projects. In reality it is as simple as creating a php file that acts as a background handler, and call it multiple times concurrently with exec().

Will have a look (after calendar module!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants