search

howardjones / network-weathermap

Network Weathermap draws diagrams from data
http://www.network-weathermap.com/
MIT License
426 stars 94 forks source link

Editor sets rrd_use_poller_output to false #164

Closed RutgerLubbers closed 5 years ago

RutgerLubbers commented 5 years ago

Given a configuration that has rrd_use_poller_output set to 1, this property is set to false (0) after an edit. (For instance 094-test.conf has this hint / property set.)

The git commit 846b5f6973a507e27a520a67e6dc02104a23053b introduced some logic to fallback to the traditional rrdtool instead of using poller output.

In RRDTool.php, line 51, a the map's context is checked to differ from the value cacti. Since the editor has the context editor the value for rrd_use_poller_output is set to false after each edit.

A bit further in the file (line 66) a map's context is checked against cli. So, would it be better to check the map's context to be equal to cli instead of not being equal to cacti?

I could create a PR for this, however I don't know how to check whether this does not reintroduce the issue that was solved by the git commit 846b5f6973a507e27a520a67e6dc02104a23053b.

howardjones commented 5 years ago

Huh, that's not great. I think the solution should be: a property on this class to store the 'effective' value of rrd_use_poller_output determined by init(), so that the readData() method can use it, without changing the original hint at all.

The context not being 'cacti' is the right way around... once we get to contexts of 'librenms' etc also, it's something that doesn't need to be adjusted all the time.

RutgerLubbers commented 5 years ago

All right, I think I can set this property in the RRDTool class. Would you like to discuss this here first, or should I create a PR and continue there?

howardjones commented 5 years ago

Go for it! It should be a pretty simple change.