Add support for batched point writing and fixed README error

[dokie]

I wanted to use the library to test batch writing so I have attempted to add this capability.
I also spotted a minor error in the README.

[mneudert]

And I think you should definitely get that feature :)

But there are some things we should probably sort out before merging this.

(After proofreading what nice wall of text will appear I split it up a bit. The first block stands on itself, the other ones are somewhat connected but addressable and debatable on their own. The final one after the thick line is a way to solve this mess.)

---

The line encoder was broken even before your fix when passing multiple points to it. Not having a real way to write multiple points just made it irrelevant.

Your fix is a nice start, but still would fail when not passing a timestamp. To really fix that problem we should probably store the created lines in a list and afterwards create the raw data using Enum.join(lines, "\n"). That would also make the String.rstrip1/ call obsolete.

For the changes to the query construction part it is purely some stylistic nitpicking.

You probably removed the def query(payload, opts \\ []) because the way you overloaded it was not fancied by the compiler. However you could just write def query(payload, opts \\ []) without a do-end-block after it. If that is the first line then query/1 should automatically call query/2 with the default value like you did in line 32. Just without having to write/spec/doc it manually :)

That definition should also be the only one with an @spec above it. Using map | [map]for the payload argument should match both cases. The type should be map as Elixir does not define a type on its own. And while dialyzer might not pick that up the "native Erlang" types should be lowercase and only when a .t (== @type) is available it should have an uppercase first letter.

Then you could simply use the guard clauses is_map/1 and is_list/1 to address them. Ideally by modifying the is_map/1 match to modify the payload to be a list.

Having a list should make the rest more obvious and clean. The call to query/2 in line 19 for constructing the default accumulator is quite hidden. Maybe 3 functions would make that more explicit, like one for "no points left", "next point in list", and a new one for "first point with empty accumulator". The whole accumulation should also be moved to a private function. No one should ever have the need to call it directly.

The test data for batch writing should use two different timestamps. Having both write at 1439587926 might store them in the wrong order (actually had that problem more than once). Incrementing one by a second would prevent the test from failing because of some weird result ordering.

And for the last: Line 14 should prepend to the list for "performance reasons". AFAIK appending is O(n) while prepending is O(1).

If you are batch writing I assume you are not talking about five but more like fifty points. Each recursion would increase the cost of appending to the list. And if one reaches a three digit count of points it might actually have a negative effect.

To still maintain the order of points a final Enum.reverse/1 in the "no points left" function should be the right thing to do. The problems that might happen when passing multiple struct points with different databases attached are something I would simply ignore for the time being...

---

As the line encoder was broken before I would go ahead and fix that on master with a test attached to it. For the other changes I could also take your commit and modify it a bit to reflect the wall of text above.

I'd leave both choices up to you and prepare a hex release in the meantime so you could use a stable release instead of some uncachable git dependency 👍

[dokie]

Thanks for the very constructive and excellent feedback. Do you want me to fix up the changes and resubmit the PR? Or you going to do the work completely?
Cheers
👌

BTW - I forgot to say, this library really helped in getting me up and running in evaluating Influxdb from Elixir to assess whether we could use it in our software, so thank-you. What are you using Influxdb for? We are taking data (high-speed pressure logging in water networks) and performing analysis upon it and wanted a simple first step at a timeseries db before perhaps jumping into say, Cassandra, which I've made heavy use of before.

[mneudert]

From a short glance it looks nice 👍

Will take a proper look later and then probably simply merge it. Any other changes (like adding a linebreak here and there) is nothing to hold it back.

I am using it for some web analytics pet project. The way you can aggregate data structured by tags just seemed right and having it only depend on go makes it easily compilable/deployable. And once the cluster features are all in it should also be able to ingest enough data for most use cases.

[dokie]

Great thanks for the feedback - I really appreciated it and fired me up to try to sort it out on the train in to work this AM!