Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miro_dietiker’s picture

Promoting :-)

mbovan’s picture

Status: Active » Needs review
FileSize
6.38 KB
31.01 KB

Adding a patch here. This was hard to test even manually so the patch does not contain tests.

Here is the screenshot of the sensor:

As a part of this issue, I created a new submodule inmail_monitoring which adds a dependency to monitoring module. It can be extended with more plugins in #2399779: Monitoring sensors for inmail.

Status: Needs review » Needs work

The last submitted patch, 2: monitor_imap_quota-2409923-2.patch, failed testing.

mbovan’s picture

Implemented FetcherBase::getQuota()

miro_dietiker’s picture

Monitoring sensors are plugins + configuration with full freedom to the user in how to use the sensors. There is no reason to add a submodule - no other module provides monitoring through a submodule.

miro_dietiker’s picture

Status: Needs review » Needs work

The patch above accidentally contains parts of the "Connect" button issue.

We usually use drop downs instead of radios.

Also, quota is more a relative thing. So the value should be percent of total.
The message can contain the effective value, but shorter please. Use "addMessage", not "setMessage".

Thus, we can use threshold exceeds for >80% usage.
Plz check how to properly register the percent value... Hope monitoring supports it. :-)

mbovan’s picture

Monitoring sensors are plugins + configuration with full freedom to the user in how to use the sensors. There is no reason to add a submodule - no other module provides monitoring through a submodule.

Wasn't there an idea to have Inmail as a clean module with no dependencies and move configuration+third party plugins to submodules (inmail_cfortune, inmail_collect, inmail_demo, inmail_mailmute...).
Anyhow, Monitoring seems like a specific module, so moved a sensor plugin it into the core (Inmail).

The patch above accidentally contains parts of the "Connect" button issue.

Hm.. Which parts?

We usually use drop downs instead of radios.

Changed to drop down.

Also, updated the status with shorter message and percentage value.

Status: Needs review » Needs work

The last submitted patch, 7: monitor_imap_quota-2409923-7.patch, failed testing.

mbovan’s picture

Updated test dependencies.

Status: Needs review » Needs work

The last submitted patch, 9: monitor_imap_quota-2409923-9.patch, failed testing.

mbovan’s picture

Removed monitoring dependency from Inmail.

miro_dietiker’s picture

Status: Needs review » Needs work
  1. +++ b/src/Plugin/monitoring/SensorPlugin/IMAPQuotaSensorPlugin.php
    @@ -0,0 +1,104 @@
    +    foreach ($deliverers as $deliverer) {
    +      // @todo: Replace with getPluginInstance() after https://www.drupal.org/node/2769617.
    +      $plugin = $deliverer_manager->createInstance($deliverer->getPluginId(), $deliverer->getConfiguration());
    

    Yeah? :-)

  2. +++ b/src/Plugin/monitoring/SensorPlugin/IMAPQuotaSensorPlugin.php
    @@ -0,0 +1,104 @@
    +      if ($plugin instanceof ImapFetcher) {
    

    Pity there is no method that provides us plugins of a specific type..?

  3. +++ b/src/Plugin/monitoring/SensorPlugin/IMAPQuotaSensorPlugin.php
    @@ -0,0 +1,104 @@
    +  public function runSensor(SensorResultInterface $result) {
    

    This needs to survive a situation when the IMAP plugin is not available on server.

  4. +++ b/src/Plugin/monitoring/SensorPlugin/IMAPQuotaSensorPlugin.php
    @@ -0,0 +1,104 @@
    +    $imap_fetcher = DelivererConfig::load($this->sensorConfig->settings['imap_fetcher']);
    

    A fetcher could be deleted (or none selected), thus you need to care about failed loads.

  5. +++ b/src/Plugin/monitoring/SensorPlugin/IMAPQuotaSensorPlugin.php
    @@ -0,0 +1,104 @@
    +      if ($usage >= $limit) {
    +        $result->setStatus(SensorResultInterface::STATUS_CRITICAL);
    +      }
    +      else {
    +        $result->setStatus(SensorResultInterface::STATUS_OK);
    +      }
    +      $result->setValue($percentage);
    

    A usage>limit is technically not possible.

    You simply setValue.

    A user can setup a threshold >90 to critical in monitoring and things are perfect and much more flexible.

mbovan’s picture

Fixed #12 1, 3, 4, 5:

#12.2: There is a getPluginType() but it only supports Inmail plugin types (analyzer/deliverer/handler). In this case "fetcher" would be a subtype of deliverer.

miro_dietiker’s picture

Status: Needs review » Fixed

OK committing this.

Did some cleanups... Tests are completely missing, added a @todo, i guess we will revisit this sensor when in use and will then check follow-ups.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.