search

ffnord / ffmap-backend

THIS PROJECT DOESN'T HAVE A MAINTAINER!
Other
20 stars 59 forks source link

Added graphite support for storage of statistical data #67

Closed mruettgers closed 8 years ago

jplitza commented 8 years ago

A general hint regarding Pull Requests on Github: It is best to develop a feature like this in an appropriately named feature branch (e.g. "graphite-support") in your fork. To update the code that you want to have merged, you can then simply modify that branch (rebase it for example), and the pull request will automatically be adjusted, with no need to open another one.

jplitza commented 8 years ago

The code looks good to me, but I think it would be much better to use the pickle protocol of graphite since the amount of data that is to be transferred could be quite big (we would roughly have 350 nodes with 15 attributes each, making up a total of over 5000 values). I realized that the pickle protocol has no advantages for many data sets, only for large data sets.

That probably eliminates the other problem I have with the plaintext version: In the unlikely case that some community or Gluon itself adds attributes with spaces in their names, this would currently break. You should probably replace spaces by an underscore when passing them to graphite.

mruettgers commented 8 years ago

Escaping of spaces is now implemented.

jplitza commented 8 years ago

Great! If you could now please squash the commits, then I'd merge it.

mruettgers commented 8 years ago

All issues have been solved. The case issue I mentioned in IRC was reported by a user working with the wrong branch in our fork.

jplitza commented 8 years ago

I renamed ts to timestamp and merged this as 84fe43f.