Skip to content

Switch from Cron to Job class - #4

Open
Massi-X wants to merge 3 commits into
FreePBX:release/16.0from
Massi-X:release/16.0
Open

Switch from Cron to Job class#4
Massi-X wants to merge 3 commits into
FreePBX:release/16.0from
Massi-X:release/16.0

Conversation

@Massi-X

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

Copy link
Copy Markdown

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.

@jfinstrom jfinstrom left a comment

Copy link
Copy Markdown

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

Massi-X commented Dec 5, 2023

Copy link
Copy Markdown
Author

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

@Massi-X

Massi-X commented Dec 5, 2023

Copy link
Copy Markdown
Author

And here we go, added missing class

@jfinstrom jfinstrom left a comment

Copy link
Copy Markdown

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

Massi-X commented Jan 30, 2024

Copy link
Copy Markdown
Author

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

@kapilgupta01
kapilgupta01 self-requested a review February 12, 2024 04:05
@kapilgupta01

Copy link
Copy Markdown
Contributor

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

Massi-X commented Feb 12, 2024

Copy link
Copy Markdown
Author

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

@kapilgupta01

Copy link
Copy Markdown
Contributor

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

jfinstrom commented Feb 12, 2024

Copy link
Copy Markdown

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

Massi-X commented Feb 12, 2024

Copy link
Copy Markdown
Author

@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