oetiker / rrdtool-1.x

RRDtool 1.x - Round Robin Database
http://www.rrdtool.org
GNU General Public License v2.0
1k stars 263 forks source link

python binding: dump() writes to stdout, instead of returning value #676

Open azrdev opened 8 years ago

azrdev commented 8 years ago

Try:

import rrdtool
xml = rrdtool.dump('some-db.rrd')

Expected: xml should contain XML file/string Actual: XML is dumped to stdout, xml is empty

oetiker commented 8 years ago

rrdtool dump actually DOES write to stdout ... in order to get the dump in memory the code of rrd_dump.c would have to be altered first!

nirgal commented 8 years ago

Maybe the python documentation could be improved:

$ pydoc rrdtool.dump

Help on built-in function dump in rrdtool:
   rrdtool.dump = dump(...)
    dump - dump an RRD to XML
    [--header|-h {none,xsd,dtd}] [--no-header]file.rrd [file.xml]
azrdev commented 8 years ago

@oetiker yes, and since this is not really useful for a library it's obviously a bug

oetiker commented 8 years ago

Are you suggesting that the dump functionality should not have been included into the python bindings. As you are expecting 'dump' to not dump into a file but into memory ?

If you are working on a patch, please make it so that it preserves the present functionality since many people my be relying on this. Just adding a memory dump function (since rrd files can be pretty large, you may want to be careful with this as it may cause undue pressure on the memory subsystem. The restore functionality for example has been re-written to use a pull parser to avoid reading in the complete rrd file as this was causing problems on some platforms)

atommaki commented 3 years ago

I agree with @azrdev, the current state is useless and confusing. (and surprisingly no change in 5+ years...). No one except a function to print on the stdout instead of returning something. If you think the memory usage can be an issue then the dump should have 2 parameters, one for the input (what is has already) and one for the output xml file.

oetiker commented 3 years ago

PR will be gladly accepted!

atommaki commented 3 years ago

While checking the code I've realized it's actually works, but the documentation is not so detailed, therefore can be misleading.

First I've tried like this:

rrdtool.dump("test.rrd")

which works (would be better it doesn't work!) the result goes to the stdout, quite useless.

But this one also works and writes the output to a file:

rrdtool.dump("test.rrd", "rrdtest.xml")

So I would say the bug is invalid, but the documentation also could be updated with more details.