Problem/Motivation
After sensor_info died, we have leftovers from previous naming sensor_info in drush and services.
We need to decide what / how these concepts are named to the outside... If we stick with sensor_info or change its name.
Proposed resolution
Decide about logic naming of the sensor_info / sensor instances exposed.
Change names, update the clients...
Remaining tasks
User interface changes
API changes
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | monitoring-2383873-sensor-config-25.patch | 19.83 KB | miro_dietiker |
| #19 | monitoring-2383873-sensor-config-18.patch | 19.03 KB | miro_dietiker |
| #12 | monitoring-2383873-sensor-info-to-config-12.patch | 10.09 KB | Anushka-mp |
| #2 | monitoring-2383873-sensor-info-to-config.patch | 8.88 KB | Anushka-mp |
Comments
Comment #1
berdirShouldn't be easy to rename the plugin id and drush command to sensor-config? We could also go with just monitoring-sensor, but given that we have sensor-result as well...
Comment #2
Anushka-mp commentedRenamed quite some stuff. from sensor-info to sensor-config
Comment #3
Anushka-mp commentedComment #4
berdirQuestion here is if this makes sense I guess, or if it should be monitoring-sensor-config, but we just have monitoring-something for all commands and they are all sensor related.
Comment #5
miro_dietikerYeah, i think we should NOT drop "sensor" just because currently all commands are about sensors.
However, with the list of sensors, we need to decide if -config (previous -info) is wanted. Logically it's not about config only, it's a list of existing sensors.
Comment #6
berdirThere is no "sensor" to drop in the drush commands? I was wondering about the opposite, *adding* it. But I think that's a different discussion and we would have to rename everything then.
RTBC then I think. Note that this will change the public URL's, so we should make sure that's what we want and are going to keep it...
Comment #7
miro_dietikerHm, i think we should discuss this.
I thought about renaming the monitoring sensor list resource from monitoring-sensor-info to monitoring-sensor instead of monitoring-sensor-config.
If you look at the whole other refactoring Sensor => SensorPlugin and Sensor => SensorConfig, my suggestion was to stay with Sensor for everything that is not assigned to these two other concepts. The discussion is wether the list resource lists Sensors or Configurations. In my opinion, It should list Sensors (Instances of the SensorPlugin) and the fact that these are Config driven is an implementation detail from the Services perspective.
Comment #8
berdirAs commented in #2384601: Rename Sensor to SensorPlugin, we should extract everything drush/services related from that patch that's not already here and do that separately.
Comment #9
Anushka-mp commentedchanged dush.inc as pointed out in #2384601: Rename Sensor to SensorPlugin.
Comment #11
berdirWe decided to rename the rest plugin to just monitoring-sensor. Also remove the Config from the resource class name then.
There are also some arguments in there, like expand on 'sensor_info', change that to 'sensor' as well.
drush.inc and and both Resource classes should no longer contain any "sensor_info" or "sensor-info", probably also no "info" at all.
Examples need to be updated.
Comment #12
Anushka-mp commentedOkay reworked, no more info or config.
Comment #13
Anushka-mp commentedComment #15
Anushka-mp commentedComment #19
miro_dietikerOK, added some further changes such as expand = sensor instead sensor_info.
Hope this passes now.
Comment #21
berdirYep, #2183983: Find hidden configuration schema issues happened, so all tests by default enforce a valid config schema.
Can be temporarily disabled by setting that flag in our base test classes to FALSE, I suggest we do that, and then work on the config schema issue, which now allows to easily validate that our config schema is correct.
Comment #23
miro_dietikerCommitted the schema unchecking. Updated the issue. Let's see what the bots think about all this.
Comment #25
miro_dietikerRetrying :-)
Comment #26
miro_dietikerChanging title...
Comment #27
miro_dietikerFixed this.