Skip to content
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

Check maintenance window time when using WebCron #32107

Closed
wants to merge 1 commit into from
Closed

Check maintenance window time when using WebCron #32107

wants to merge 1 commit into from

Conversation

k-popov
Copy link
Contributor

@k-popov k-popov commented Apr 23, 2022

The patch moves maintenance window check earlier so that
maintenance window was respected when using both system cron
and WebCron. Before the change webcron used to select
only time-sensitive tasks.

Signed-off-by: Kirill Popov kirill.s.popov@gmail.com

The patch moves maintenance window check earlier so that
maintenance window was respected when using both system cron
and WebCron. Before the change webcron used to select
**only** time-sensitive tasks.

Signed-off-by: Kirill Popov <kirill.s.popov@gmail.com>
@szaimen szaimen added bug 3. to review Waiting for reviews labels Apr 23, 2022
@szaimen szaimen added this to the Nextcloud 25 milestone Apr 23, 2022
@szaimen szaimen requested review from nickvergessen, a team, icewind1991, blizzz and juliushaertl and removed request for a team April 23, 2022 14:04
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Tkme sensitivity should only be considered for cron, not for web or ajax

@@ -167,8 +168,9 @@
} else {
// Work and success :-)
$jobList = \OC::$server->getJobList();
$job = $jobList->getNext();
$job = $jobList->getNext($onlyTimeSensitive);
Copy link
Member

Choose a reason for hiding this comment

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

The actual fix would be to just inject false here or change the default of the method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please comment why?
I'm running NextCloud in Kubernetes and thus I can't use system cron daemon. Having Kubernetes' CronJob is a solution - it runs periodically and "curl's" cron.php. In this setup I still have short background jobs and time-sensitive ones which I want to run during maintenance hours.

Copy link
Member

Choose a reason for hiding this comment

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

Explanation in #30899 (comment)

@k-popov
Copy link
Contributor Author

k-popov commented Apr 24, 2022

I've opened #32109 with argument default change.

@k-popov k-popov closed this Apr 24, 2022
@szaimen szaimen removed this from the Nextcloud 25 milestone Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants