should SDK terminate when bad config files are given

[edaniszewski]

I can't remember if this question was asked before, or what the answer to it was.

with an intentionally incorrect device config from the logging branch (https://github.com/vapor-ware/synse-sdk/pull/150)

INFO[0000] Loading plugin config from file              
INFO[0000] Adding configuration path: /etc/synse/plugin 
INFO[0000] Adding configuration path: /Users/edaniszewski/.synse/plugin 
INFO[0000] Adding configuration path: .                 
DEBU[0000] Setting v1 Plugin config defaults.           
INFO[0000] Starting plugin run                          
DEBU[0000] devicesFromConfig start                      
DEBU[0000] ParseDeviceConfig start                      
DEBU[0000] Searching config/device for device configurations. 
DEBU[0000] Found configuration files: [0xc42016bc70 0xc42016bd40 0xc42016be10 0xc42016bee0 0xc420198000 0xc4201980d0] 
DEBU[0000] Reading file: config/device/airflow.yaml     
ERRO[0000] Failed to Unmarshal config data for config/device/airflow.yaml: yaml: line 7: mapping values are not allowed in this context 
ERRO[0000] Failed to get configuration version for file config/device/airflow.yaml: yaml: line 7: mapping values are not allowed in this context 
ERRO[0000] error when parsing device configs: yaml: line 7: mapping values are not allowed in this context 
ERRO[0000] Failed to register devices from files: yaml: line 7: mapping values are not allowed in this context 
INFO[0000] Plugin Info:                                 
INFO[0000]  Name:        emulator                       
INFO[0000]  Version:     1.0                            
INFO[0000]  SDK Version: 0.5.0                          
INFO[0000]  Git Commit:  4eb1c1e                        
INFO[0000]  Git Tag:     -                              
INFO[0000]  Go Version:  go1.9.1                        
INFO[0000]  Build Date:  2018-03-24T16:21:08            
INFO[0000]  OS:          darwin                         
INFO[0000]  Arch:        amd64                          
INFO[0000] Plugin Config:                               
INFO[0000] {
  "Name": "emulator",
  "Version": "1",
  "Debug": true,
  "Settings": {
    "Mode": "parallel",
    "Read": {
      "Enabled": true,
      "Interval": "1s",
      "Buffer": 100
    },
    "Write": {
      "Enabled": true,
      "Interval": "2s",
      "Buffer": 100,
      "Max": 100
    },
    "Transaction": {
      "TTL": "5m"
    }
  },
  "Network": {
    "Type": "unix",
    "Address": "emulator.sock"
  },
  "AutoEnumerate": [],
  "Context": {},
  "Limiter": null
} 
INFO[0000] Registered Devices:                          
INFO[0000] --------------------------------             
INFO[0000] Initializing DataManager goroutines..        
INFO[0000] plugin reads enabled - starting the read goroutine 
INFO[0000] plugin writes enabled - starting the write goroutine 
INFO[0000] starting data updater                        
INFO[0000] DataManager initialization complete.         
INFO[0000] Starting gRPC server                         
INFO[0000] grpc server listening on unix: /tmp/synse/procs/emulator.sock 
INFO[0000] serving...   

so it logs errors for the config, but then proceeds to serve. bad configs generally means things won't work, or at least things won't work entirely as desired, so maybe a bad config should terminate the plugin.

[MatthewHink]

It looks like this error: ERRO[0000] Failed to Unmarshal config data for config/device/airflow.yaml: yaml: line 7: mapping values are not allowed in this context Is caused by json/yaml that doesn't parse.

One way to test for this is to add a test for each repo that parses the json/yaml and passes/fails based on whether or not it parses.

Another way is to write one test that enumerates / synches all repos, finds and parses all json/yaml files, and reports the parse errors. Second way is likely better to avoid rewriting the test all over (extensibility).

---

I would prefer to terminate (not serve) if the config cannot possibly work at all. I spend too much time with bad configurations and it's often unclear what they should be (What file is being parsed, true line number of the error, etc.)

If there is a device missing, it could be out for service, network cable could have physically fallen out etc. In that case we should flag an error, move along, and just work. This is a different case than a config file parse error.

---

Side note / Separate issue: This trace: DEBU[0000] Found configuration files: [0xc42016bc70 0xc42016bd40 0xc42016be10 0xc42016bee0 0xc420198000 0xc4201980d0] Is lacking and something I added out of desperation. Ideally we trace the file names rather than the pointer/memory addresses. It's really hard to tell what is happening with the configuration code.

[edaniszewski]

This was fixed a while ago:

➜ ./plugin 
INFO[0000] Loading plugin config from file              
INFO[0000] Adding configuration path: /etc/synse/plugin 
INFO[0000] Adding configuration path: /Users/edaniszewski/.synse/plugin 
INFO[0000] Adding configuration path: .                 
ERRO[0000] Failed to read in plugin config : While parsing config: yaml: line 5: did not find expected key 
ERRO[0000] Failed to parse versioned plugin config: While parsing config: yaml: line 5: did not find expected key 
ERRO[0000] Failed to load plugin config from file: While parsing config: yaml: line 5: did not find expected key 
2018/04/27 14:38:47 While parsing config: yaml: line 5: did not find expected key
➜ echo "$?"
1