ported all Checks from LiipMonitor (fixes #3) #10
Conversation
|
There is some overlap with #8 |
| { | ||
| foreach ($this->extensions as $extension) { | ||
| if (!extension_loaded($extension)) { | ||
| return new Failure(sprintf('Extension %s not loaded', $extension)); |
There was a problem hiding this comment.
wouldn't it be better to report all missing extensions instead of only the first one ? It would be consistent with WritableDirectoryCheck
|
Oh no @lsmith77. |
| use ZendDiagnostics\Result\Failure; | ||
| use ZendDiagnostics\Result\Success; | ||
|
|
||
| class DiscUsageCheck extends AbstractCheck |
There was a problem hiding this comment.
Typo in class name (and file name).
There was a problem hiding this comment.
Please drop Check suffix from class names because we already have a NS for this:
ZendDiagnostics\Check\DiskSpace
// instead of
ZendDiagnostics\Check\DiskSpaceCheck
|
not sure if i will have time this week. if you have time. feel free to do any changes in this branch/PR |
| $memcache = new \Memcache(); | ||
| $memcache->addServer($this->host, $this->port); | ||
| $stats = @$memcache->getExtendedStats(); | ||
| $available = $stats[$this->host . ':' . $this->port] !== false; |
There was a problem hiding this comment.
This is hairy! In case the \Memcache class is missing it will crash. In case addServer doesn't work as it's supposed to, it will crash. In case getExtendedStats() doesn't work as expected, you'll get a php notice (because $array[] will be missing a key).
I believe it should start with checking if MemCache is available at all and throw a Failure otherwise... then it should be able to work around problems with incompatible versions and data coming from methods...
|
@lsmith77 Ok, I'll take care of those changes tomo. I'll also show you my alternative implementation of |
ghost
assigned
|
As per my previous comment, I've created PR #13 for |
| @@ -0,0 +1,46 @@ | |||
| <?php | |||
There was a problem hiding this comment.
so this check is redundant and should therefore be removed from the PR?
There was a problem hiding this comment.
I wonder if we could port the "percent" functionality into DiskFree - i.e. if the suffix is % then it'd perform a relative calculation. How do you like this idea ?
- Refactor class names (remove "Check" suffix). - Remove Check\WritableDirectory as it overlaps Check\DirWritable. - Replace occurrences of "Disc" with "Disk". - Remove getName() methods, as they do not add any value to default implementation of AbstractCheck::getName() and ::getLabel().
percentage support will be ported to the existing one in a separate PR
|
merge? |
|
What's the coverage % ? |
|
no clue .. I guess we can add |
|
|
ported all Checks from LiipMonitor (fixes #3)
|
Can you add travis on master? Artur Bodera On 24 sie 2013, at 17:24, Lukas Kahwe Smith notifications@github.com no clue .. I guess we can add --coverage-text to travis .. actually us — |
|
all ready have a .travis.yml config file in master. i can add the code coverage option. however i cannot enable the travis hook. only the owner can do this. |
|
Enabled. |
No description provided.