Open MegaphoneJon opened 9 years ago
I like it. My only concern for adding the other checks to system.check is that at the moment there's no way to filter system.check messages for severity. I'd want to be able to say that message X is a warning and message Y is critical--at the moment all we can do is key all the known ones in the plugin processing the messages.
Also, it would be nice to have purely informational requests be checkable by CiviMonitor but not result in a ton of messages for the admin when they log into CiviCRM. For example, a new check shows what outbound email system is in effect (so we can keep track of stuff like that internally for our clients), and it would be great to put that into the extension, but at most, that should be the equivalent of Drupal telling you what PHP version you're using.
This should probably be continued as a conversation at the sprint (will you be there?) but my thinking when I posted on civicrm-partners about a status page was:
So agreed that the switch to using System.check should happen after such changes took place - the status page is my preferred sprint project. However, as it stands, anything(?) in System.check IS what I'd consider critical-worthy.
Of course, I probably won't get to write my PR pre-CiviCon anyway...so let's hold this for a discussion with Tim maybe?
Sounds perfect. I'd be happy to spend sprint time with you on status stuff, too.
On Wed, Apr 8, 2015 at 4:54 PM, Jon notifications@github.com wrote:
This should probably be continued as a conversation at the sprint (will you be there?) but my thinking when I posted on civicrm-partners about a status page was:
- We'd implement a severity for status checks, similar to Drupal's hook_requirements.
- We'd phase out the notification bubbles for each failed status check in favor of something more like Drupal's approach - a single bubble referring folks to a status page.
So agreed that the switch to using System.check should happen after such changes took place - the status page is my preferred sprint project. However, as it stands, anything(?) in System.check IS what I'd consider critical-worthy.
Of course, I probably won't get to write my PR pre-CiviCon anyway...so let's hold this for a discussion with Tim maybe?
— Reply to this email directly or view it on GitHub https://github.com/aghstrategies/com.aghstrategies.civimonitor/issues/2#issuecomment-91034238 .
Andrew Hunt AGH Strategies (202) 248-6400 phone (202) 521-1363 fax aghstrategies.com
Excellent. Also - it looks like Tim's PR would modify showPeriodicAlerts to only warn on WARNING or above: https://github.com/totten/civicrm-core/commit/8369a69e3ea114c6f925e52bf89483e6b1e1c9c5
Actually got into doing a bunch this morning, so I went ahead and added the system.check: https://github.com/aghstrategies/com.aghstrategies.civimonitor/commit/6e1464f56f8b9a68a1c35c6a0bb698fda3f92dff
2 thoughts:
The advantages to this approach are:
As a note, this is very similar to how the Drupal Nagios module is architected.