Skip to content

Log jobCode when writing to log with no callback for easier debug#19541

Merged
magento-engcom-team merged 2 commits intomagento:2.3-developfrom
cscd98:logging/cronQueue
Dec 9, 2018
Merged

Log jobCode when writing to log with no callback for easier debug#19541
magento-engcom-team merged 2 commits intomagento:2.3-developfrom
cscd98:logging/cronQueue

Conversation

@cscd98
Copy link
Copy Markdown
Contributor

@cscd98 cscd98 commented Dec 4, 2018

Description (*)

When throwing a new exception with no call back, there is no information on which job it is related to. This makes it difficult to debug the issue.

Manual testing scenarios (*)

  1. Have a job code in cron_schedule with an invalid call back

Contribution checklist (*)

  • [] Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Copy Markdown
Contributor

magento-cicd2 commented Dec 4, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @craigcarnell. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@orlangur orlangur self-assigned this Dec 4, 2018
if (!isset($jobConfig['instance'], $jobConfig['method'])) {
$schedule->setStatus(Schedule::STATUS_ERROR);
throw new \Exception('No callbacks found');
throw new \Exception('No callbacks found for cron job %s', $jobCode);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you miss sprintf call here? Better use "This is $string".

Also, please provide more detailed steps to reproduce.

Copy link
Copy Markdown
Contributor Author

@cscd98 cscd98 Dec 4, 2018

Choose a reason for hiding this comment

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

@orlangur good spot :)

If you have a database converted from M1 to M2, certain crons may no longer exist.

For example: crontab/jobs/catalog_price_rule_update/schedule/cron_expr

However, there is no call back for this cron expr.

So magento error logs just contain 'No callbacks found' for several items. With this code, it now shows which cron job can be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@orlangur re pushed with sprintf added

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.1 milestone Dec 4, 2018
@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-3615 has been created to process this Pull Request

@magento-engcom-team
Copy link
Copy Markdown
Contributor

@craigcarnell thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @craigcarnell. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants