feature: Added app_state support for applications #5068 #6061

Merged
merged 11 commits into from Mar 11, 2017

Conversation

Projects
None yet
5 participants
@laf
Member

laf commented Mar 2, 2017

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.

  • Have you signed the Contributors agreement - please do NOT submit a pull request unless you have (signing the agreement in the same pull request is fine). Your commit message for signing the agreement must appear as per the docs.
  • Have you followed our code guidelines?

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

Fixes: #5068

Application support needs some major loving and a rewrite, for now this will provide a status + prev_status to be useful for alerting.

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Mar 2, 2017

Thank you for submitting a PR @laf! We have found the following @murrant, @tuxis-ie and @f0o based on the history of these files to review this PR.

Thank you for submitting a PR @laf! We have found the following @murrant, @tuxis-ie and @f0o based on the history of these files to review this PR.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

@laf laf added the Schema label Mar 2, 2017

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 7, 2017

Member

Rebased

Member

laf commented Mar 7, 2017

Rebased

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
includes/polling/functions.inc.php
+/**
+ * Update the application status and output in the database.
+ *
+ * @param array $device

This comment has been minimized.

@murrant

murrant Mar 8, 2017

Member

Why do we need $device?

@murrant

murrant Mar 8, 2017

Member

Why do we need $device?

This comment has been minimized.

@laf

laf Mar 8, 2017

Member

Good spot, I've made more use of it by adding it to the dbUpdate call where it should have been included anyway.

@laf

laf Mar 8, 2017

Member

Good spot, I've made more use of it by adding it to the dbUpdate call where it should have been included anyway.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
includes/polling/functions.inc.php
@@ -596,5 +596,5 @@ function update_applications($device, $app, $response)
if ($data['app_state'] != $app['app_state']) {
$data['app_state_prev'] = $app['app_state'];
}
- dbUpdate($data, 'applications', '`app_id` = ?', array($app['app_id']));
+ dbUpdate($data, 'applications', '`app_id` = ? AND `device_id` = ?', array($app['app_id'], $device['device_id']));

This comment has been minimized.

@murrant

murrant Mar 10, 2017

Member

app_id is unique, why do we need to check device_id too?

@murrant

murrant Mar 10, 2017

Member

app_id is unique, why do we need to check device_id too?

This comment has been minimized.

@laf

laf Mar 11, 2017

Member

Being cautious?

@laf

laf Mar 11, 2017

Member

Being cautious?

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Mar 11, 2017

The inspection completed: 10 new issues, 1 updated code elements

The inspection completed: 10 new issues, 1 updated code elements

@murrant murrant merged commit 66d1006 into librenms:master Mar 11, 2017

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@laf laf deleted the laf:issue-5068 branch Mar 11, 2017

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