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.

Sign up for GitHub

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

Jump to bottom

cron.php calls occ system:cron as fallback #36221

Merged
merged 1 commit into from
Sep 26, 2019
Merged

cron.php calls occ system:cron as fallback #36221

merged 1 commit into from
Sep 26, 2019

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Sep 26, 2019 •
edited
Loading

Description

cron.php calls the occ system:cron command as fallback

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@DeepDiver1975 DeepDiver1975 self-assigned this Sep 26, 2019
PVince81
PVince81 reviewed Sep 26, 2019
View reviewed changes
cron.php Outdated
@@ -40,4 +40,8 @@
}

echo 'Please use ./occ system:cron' . PHP_EOL;
exit(1);
$return = system('./occ system:cron');
Copy link
Contributor

Choose a reason for hiding this comment

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

this implies that PHP is allowed to call system functions

is there maybe another way where we instantiate it through Symfony like we do for unit tests ?

Copy link
Member Author

Choose a reason for hiding this comment

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

this implies that PHP is allowed to call system functions

at this level where php scripts are called from crontab we can assume that we can call system functions.

Adding more complexity here by using symfony is an overkill from my perspective

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, so assuming that the php.ini for the cli is more permissive

PVince81
PVince81 approved these changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81
Copy link
Contributor

@micbar

@micbar micbar added this to the development milestone Sep 26, 2019
micbar
micbar requested changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@micbar micbar left a comment

Choose a reason for hiding this comment

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

Could you push the changes?

cron.php Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 26, 2019 •
edited
Loading

Codecov Report

Merging #36221 into release-10.3.0 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           release-10.3.0   #36221   +/-   ##
===============================================
  Coverage              54%      54%           
===============================================
  Files                  63       63           
  Lines                7408     7408           
  Branches             1309     1309           
===============================================
  Hits                 4001     4001           
  Misses               3021     3021           
  Partials              386      386
Flag Coverage Δ
#javascript 54% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07bc371...a0d4c20. Read the comment docs.

@micbar micbar merged commit 17ba0df into release-10.3.0 Sep 26, 2019
@delete-merged-branch delete-merged-branch bot deleted the bugfix/cron.php-call-occ-cron branch September 26, 2019 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@micbar micbar micbar approved these changes

@PVince81 PVince81 PVince81 approved these changes

Assignees

@DeepDiver1975 DeepDiver1975

Labels
3 - To Review
Projects
None yet
Milestone
development
Development

Successfully merging this pull request may close these issues.
4 participants
@DeepDiver1975 @PVince81 @micbar @phil-davis