Skip to content
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

Fixes/msgpack default #783

Open
jgspiro wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fixes/msgpack default #783

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9b08282
Fix : uninitialized data using invalid protocol.
jgspiro Feb 3, 2020
4bbe67b
Unit test both with and without msgpack. Retention policy replication…
jgspiro Feb 4, 2020
088da3c
Fix client tests that are failing.
jgspiro Feb 4, 2020
065ed44
Make msgpack default option, fix demonstrator test.
jgspiro Feb 21, 2020
9a8b66d
Fix server test test_write_points_mixed_type
jgspiro Feb 24, 2020
51d0213
Remove check for JSON result in server test test_write_points_mixed_type
jgspiro Feb 24, 2020
44a26eb
Add use_msgpack docstring.
jgspiro Feb 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix server test test_write_points_mixed_type
  • Loading branch information
@jgspiro
jgspiro committed Feb 24, 2020
commit 9a8b66dc112942c24f6a13110d0e778f54c432a0
65 changes: 42 additions & 23 deletions influxdb/tests/server_tests/client_test_with_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,30 +373,49 @@ def test_write_points_mixed_type(self):
rsp1 = client_1.query('SELECT * FROM cpu_load_short_mixed')
rsp2 = client_2.query('SELECT * FROM cpu_load_short_mixed')

for res in [rsp1, rsp2]:
self.assertNotEqual(
list(res),
[[
{'value': 0.64,
'time': '2009-11-10T23:00:00Z',
"host": "server01",
"region": "us-west"},
{'value': 0.65,
'time': '2009-11-10T23:01:00Z',
"host": "server01",
"region": "us-west"},
{'value': 0.35,
'time': '2009-11-10T23:02:00Z',
"host": "server01",
"region": "us-west"},
{'value': 1.0,
'time': '2009-11-10T23:03:00Z',
"host": "server01",
"region": "us-west"}
]]
)
self.assertEqual(
list(rsp1),
[[
{'value': 0.64,
'time': '2009-11-10T23:00:00Z',
"host": "server01",
"region": "us-west"},
{'value': 0.65,
'time': '2009-11-10T23:01:00Z',
"host": "server01",
"region": "us-west"},
{'value': 0.35,
'time': '2009-11-10T23:02:00Z',
"host": "server01",
"region": "us-west"},
{'value': 1,
Copy link
Contributor

@lovasoa lovasoa Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that influxdb returns 1 instead of 1.0 here is a bug. I don't think you should assert that the bug is present. A future version of influxdb could fix that bug, and you wouldn't want the tests too break then. I think you should just remove this assertion.

Copy link
Contributor Author

@jgspiro jgspiro Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue you mentioned was closed without a fix, so I doubt this will change soon. It is a result of the default json marshaling in GoLang (they could probably work around it by customizing).

In contrast to you I believe it would be a good thing that the test would fail if the expected result would suddenly change (since the influx versions used to test are pinned it should not).

Copy link
Contributor

@lovasoa lovasoa Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be a good thing that the test would fail if the expected result would suddenly change

Yes, but the expected result in this case is 1.0, not 1, since the database contains a float. The problem is that the library does not return the expected result. I don't think this buggy behavior should be enforced by a test.

Copy link
Contributor Author

@jgspiro jgspiro Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both are the same Number in JSON, which is the reason the issue was closed in InfluxDB.
I did remove the JSON result from the test - altough I still think it should be there, because it is a way to document the current and future behaviour.

Copy link
Contributor

@lovasoa lovasoa Feb 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, both are the same Number in JSON, but that does not make this less of a bug 😛 Python has a different type for int and float, and so does InfluxDB.
And it is indeed a good idea to document the problematic behavior. Maybe you could add something to the doc comments ?

Copy link
Contributor Author

@jgspiro jgspiro Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed I forgot to add the use_msgpack param docstring to InfluxDBClient doc, so I just pushed a commit documenting that paramter.

Is there a specific place you want me to describe the msgpack/Json difference?

Copy link
Contributor

@lovasoa lovasoa Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc string of the influxdb client seems like a good place to me.
Also, I'm not sure it's clear: I do not have commit rights on this repository. None of the persons with commit rights are active on the repository anymore.

'time': '2009-11-10T23:03:00Z',
"host": "server01",
"region": "us-west"}
]]
)

self.assertEqual(rsp1, rsp2)
self.assertEqual(
list(rsp2),
[[
{'value': 0.64,
'time': '2009-11-10T23:00:00Z',
"host": "server01",
"region": "us-west"},
{'value': 0.65,
'time': '2009-11-10T23:01:00Z',
"host": "server01",
"region": "us-west"},
{'value': 0.35,
'time': '2009-11-10T23:02:00Z',
"host": "server01",
"region": "us-west"},
{'value': 1.0,
'time': '2009-11-10T23:03:00Z',
"host": "server01",
"region": "us-west"}
]]
)

def test_write_points_check_read(self):
"""Test writing points and check read back."""
Expand Down