Problem/Motivation

The drush command is broken.

$ drush monitoring-sensor-config
PHP Fatal error:  Call to a member function getLabel() on a non-object in /usr/local/var/www/d8.dev/www/modules/monitoring/monitoring.drush.inc on line 147

And more fun to enjoy...

$ drush monitoring-purge-settings node_new_all
exception 'Drupal\Core\Config\ImmutableConfigException' with message 'Can not set values on immutable configuration monitoring.settings:node_new_all. Use              [error]
\Drupal\Core\Config\ConfigFactoryInterface::getEditable() to retrieve a mutable configuration object' in
/usr/local/var/www/d8.dev/www/core/lib/Drupal/Core/Config/ImmutableConfig.php:34

$ drush monitoring-disable node_new_all
The sensor All new nodes is currently disabled.

(Although the sensor was enabled and still is.)

Proposed resolution

Fix it.
And check all other drush commands.

Possibly more are broken?

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#3 drush_broken-2530372-3.patch3.65 KBmbovan
#3 drush_broken-2530372-3-interdiff.txt3.3 KBmbovan
#1 drush_broken-2530372-1.patch3.05 KBmbovan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mbovan’s picture

mbovan CreditAttribution: mbovan at MD Systems GmbH commented
Status: Active » Needs review
FileSize
drush_broken-2530372-1.patch3.05 KB

Fixed monitoring-sensor-config, monitoring-purge-settings and monitoring-disable commands.

Changed a message for already disabled sensors to match "enabled" sensor message.

Added check for non existing sensors in monitoring-purge-settings. Otherwise, we can get a message that settings are purged for non-existent sensor.

Other drush commands work as expected.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work 
  1. +++ b/monitoring.drush.inc
@@ -141,13 +141,13 @@ function monitoring_drush_sensor_config_all() {
+  /** @var \Drupal\monitoring\Entity\SensorConfig $sensors_config */
   foreach ($sensors_config_all as $name => $sensors_config) {
     $rows[] = array(
-      $config->getLabel(),
+      $sensors_config->getLabel(),
       $name,
-      $config->getCategory(),
-      ($config->isEnabled() ? t('Yes') : t('No')),
+      $sensors_config->getCategory(),
+      ($sensors_config->isEnabled() ? t('Yes') : t('No')),

    $sensors_config is a bit weird.

    What about $sensor_config_list and $sensor_config as the variable names? 

  2. +++ b/monitoring.drush.inc
@@ -372,12 +372,12 @@ function monitoring_drush_disable($sensor_name) {
     $sensor_config = $sensor_manager->getSensorConfigByName($sensor_name);
-    if (!$sensor_config->isEnabled()) {
-      $sensor_manager->enableSensor($sensor_name);
+    if ($sensor_config->isEnabled()) {
+      $sensor_manager->disableSensor($sensor_name);
       drush_log(dt('The sensor @name was disabled.', array('@name' => $sensor_config->getLabel())), 'ok');
     }
     else {
-      drush_log(dt('The sensor @name is currently disabled.', array('@name' => $sensor_config->getLabel())), 'warning');
+      drush_log(dt('The sensor @name is already disabled.', array('@name' => $sensor_config->getLabel())), 'warning');
     }

    Was that broken at some point during the port or is it also like this in the 7.x-1.x branch? 

  3. +++ b/monitoring.drush.inc
@@ -392,16 +392,25 @@ function monitoring_drush_disable($sensor_name) {
 function monitoring_drush_purge_settings($sensor_name = NULL) {
+  $sensor_manager = monitoring_sensor_manager();
+  $settings = \Drupal::configFactory()->getEditable('monitoring.settings');
   if (empty($sensor_name)) {
     if (drush_confirm(dt('Do you want to purge all sensor settings?'))) {
-      foreach (monitoring_sensor_manager()->getAllSensorConfig() as $sensor_name => $sensor_config) {
-        \Drupal::config('monitoring.settings')->set($sensor_name, array())->save();
+      foreach ($sensor_manager->getAllSensorConfig() as $sensor_name => $sensor_config) {
+        $settings->set($sensor_name, array())->save();
       }
       drush_print(dt('Purged settings for all sensors'));
     }

    This makes no sense anymore. This configuration doesn't exist anymore, sensors are config entities. Let's just remove this command.

mbovan’s picture

mbovan CreditAttribution: mbovan at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
drush_broken-2530372-3-interdiff.txt3.3 KB
drush_broken-2530372-3.patch3.65 KB

1. Renamed.

2. It seems that is the same error in 7.x-1.x. I'll open a 7.x-1.x issue?

3. Removed.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

2. Yes, separate issue makes sense, since this also does a lot of other things, so it doesn't really make sense to backport this issue.

mbovan’s picture

mbovan CreditAttribution: mbovan at MD Systems GmbH commented

Ok, here is the 7.x-1.x issue: #2532040: Fix disabling sensors using drush.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Thanks, committed.

  • Berdir committed 0937612 on 8.x-1.x authored by mbovan 
    Issue #2530372 by mbovan: Fixed broken drush commands, remove purge-...
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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