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

Fix bug of alerting by ping.php #9311

Merged
merged 8 commits into from Oct 12, 2018

Conversation

Projects
None yet
4 participants
@DR3EVR8u8c
Contributor

DR3EVR8u8c commented Oct 9, 2018

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

please refer the Help ticket for more information:

https://community.librenms.org/t/please-help-with-for-fast-ping-checks-problems/5586

Bug description:
the PingCheck.php saves the device status after it run rules. Therefore, the rules cannot be detected correctly as the devices table hasn’t been updated. While Poller.php update the devices status in db before it run the rules detection.

DR3EVR8u8c added some commits May 15, 2018

Merge pull request #1 from librenms/master
update to upstream
Fix bug of alert by ping.php.
Fix bug of alert by ping.php.
Please refer to the Help ticket for more information:
https://community.librenms.org/t/please-help-with-for-fast-ping-checks-problems/5586
@TheGreatDoc

If you set `$device->save()' before the RunRules, Its needed also after it? I think you could remove it.

laf added some commits Oct 9, 2018

@laf

This comment has been minimized.

Member

laf commented Oct 9, 2018

@murrant Does this look like the correct way to fix this?

@murrant

This comment has been minimized.

Member

murrant commented Oct 11, 2018

@laf no, this is not correct. After the device model has been saved, it is no longer dirty.

It is correct to just move the save call inside the isDirty check right before RunRules().

@murrant

This comment has been minimized.

Member

murrant commented Oct 11, 2018

Actually, because of last_ping. We can't do it that way.

@murrant

LGTM

@murrant murrant added this to the 1.44 milestone Oct 11, 2018

@DR3EVR8u8c DR3EVR8u8c closed this Oct 11, 2018

@laf laf reopened this Oct 12, 2018

@murrant murrant merged commit 36e2467 into librenms:master Oct 12, 2018

3 checks passed

WIP ready for review
Details
codeclimate All good!
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment