Isilon / isilon_data_insights_connector

Connector to fetch stats from OneFS and push them to InfluxDB/Grafana
MIT License
76 stars 36 forks source link

DerivedStatInput code fails pylint, unclear what it's supposed to be doing #93

Closed tenortim closed 4 years ago

tenortim commented 4 years ago

isi_data_insights_daemon.py:162: No value for argument 'field' in method call

Not clear that this code has ever been tested (multiple fields don't show up in the sample config) and unclear what it's supposed to be doing. The (ab)use of argument packing/unpacking is ugly and seemingly unnecessary (_stat_fields is already a class attribute so it's not obvious why it's being passed as an argument). At first glance, if there are multiple fields, we'll recurse through them and ultimately only return the last one one (AFAICT).

tenortim commented 4 years ago

So I believe that pylint is actually wrong here, but it's wrong because of a somewhat abusive code pattern (Python mantra, explicit is better than implicit). I believe the code is using the "*args" pattern to "peel off" indirection one level at a time, but as a static analyser, pylint has no way to tell that this is legitimate. The code should explicitly work its way through the list/dict and call _lookup with the correct number of arguments in all cases.

A simple workaround is to provide a default value for field to prevent the pylint complaint.

tenortim commented 4 years ago

Fixed in PR #97